Improve calculation of buildPath include and exclude paths by PeterJudgeZA · Pull Request #469 · kenherring/ablunit-test-runner

PeterJudgeZA · 2026-01-03T03:14:03Z

Per issue #448

Copilot

Pull request overview

Fixes test discovery issues in multi-project workspaces (issue #448) by improving how buildPath include/exclude globs from openedge-project.json are converted into VSCode RelativePatterns, and adds debug logging/tests to validate matching behavior.

Changes:

Adjust buildPath include/exclude pattern construction to use workspace.asRelativePath(..., false) for better multi-root workspace compatibility.
Add a new parsing test + fixture for comma-delimited buildPath includes/excludes.
Enhance debug logging when a file does not match any include/exclude pattern.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 6 comments.

File	Description
test_projects/BuildPathParser/openedge-project.test.json	Adds a minimal openedge-project fixture with buildPath includes/excludes for testing.
test/parse/BuildPathParser.test.ts	Adds unit test coverage for `getBuildPathPatterns()` output patterns.
src/parse/OpenedgeProjectParser.ts	Updates logic that converts buildPath includes/excludes into `RelativePattern`s.
src/extension.ts	Adds extra debug logging for non-matching glob patterns during file inclusion checks.

Copilot · 2026-03-29T01:09:26Z

 		const lines = FileUtils.readLinesFromFileSync(FileUtils.toUri(buildPath.includesFile))
-		includes.push(...lines.map(p => new RelativePattern(workspaceFolder, workspace.asRelativePath(buildPath.path) + '/' + p)))
+		includes.push(...lines.map(p => new RelativePattern(workspaceFolder, workspace.asRelativePath(`${buildPath.pathUri.fsPath}/${p}`, false))))
 	} else {
 		for (const p of buildPath.includes?.split(',') ?? []) {
-			includes.push(new RelativePattern(workspaceFolder, workspace.asRelativePath(buildPath.pathUri) + '/' + p))
+			includes.push(new RelativePattern(workspaceFolder, workspace.asRelativePath(`${buildPath.pathUri.fsPath}/${p}`, false)))


Patterns are built via ${buildPath.pathUri.fsPath}/${p}. Because fsPath uses OS-specific separators (backslashes on Windows), concatenating with '/' can yield mixed separators and broken glob matching. Prefer Uri.joinPath(buildPath.pathUri, p.trim()) (then workspace.asRelativePath(..., false)) and skip empty entries.

Copilot · 2026-03-29T01:09:26Z

 		const lines = FileUtils.readLinesFromFileSync(FileUtils.toUri(buildPath.excludesFile))
-		excludes.push(...lines.map(p => new RelativePattern(workspaceFolder, workspace.asRelativePath(buildPath.path) + '/' + p)))
+		excludes.push(...lines.map(p => new RelativePattern(workspaceFolder, workspace.asRelativePath(`${buildPath.pathUri.fsPath}/${p}`, false))))
 	} else {
 		for (const p of buildPath.excludes?.split(',') ?? []) {
-			excludes.push(new RelativePattern(workspaceFolder, workspace.asRelativePath(buildPath.pathUri) + '/' + p))
+			excludes.push(new RelativePattern(workspaceFolder, workspace.asRelativePath(`${buildPath.pathUri.fsPath}/${p}`, false)))


Excludes patterns are also built using ${buildPath.pathUri.fsPath}/${p}, which can produce mixed separators on Windows and break minimatch. Use Uri.joinPath(buildPath.pathUri, p.trim()) (plus workspace.asRelativePath(..., false)) and filter out blank patterns before creating RelativePatterns.

Copilot · 2026-03-29T01:09:26Z

@@ -0,0 +1,41 @@
+import { getDLC, getOEVersion, getOpenEdgeProfileConfig, getBuildPathPatterns } from 'parse/OpenedgeProjectParser'


Unused imports (getDLC, getOEVersion) add noise and can trigger lint failures when running npm run lint. Remove them or use them in the test.

Suggested change

import { getDLC, getOEVersion, getOpenEdgeProfileConfig, getBuildPathPatterns } from 'parse/OpenedgeProjectParser'

import { getOpenEdgeProfileConfig, getBuildPathPatterns } from 'parse/OpenedgeProjectParser'

Copilot · 2026-03-29T01:09:26Z

+suiteSetup('suiteSetup', async () => {
+	await suiteSetupCommon()
+})
+


This suite calls suiteSetupCommon(), which starts a log group, but there is no corresponding suiteTeardown to call log.group.end(). Add a suiteTeardown (as done in other parse tests) to avoid leaking log groups across suites in CI output.

Suggested change

suiteTeardown('suiteTeardown', () => {

log.group.end()

})

Copilot · 2026-03-29T01:09:27Z

+	for (const b of config.buildPath.filter(b => b.type == 'source') ?? []) {
+		const buildPathPatterns = getBuildPathPatterns(workspaceFolder, b)


workspaceFolder can be undefined (other parse tests guard this). Add an explicit check before passing it into getBuildPathPatterns(...) so test failures are clearer and don't surface as a runtime TypeError.

Copilot · 2026-03-29T01:09:27Z

+	let allPatterns = ''
 	for (const pattern of patterns.map(pattern => pattern.pattern)) {
 		if (minimatch(relativePath, pattern, { magicalBraces: true })) {
 			log.debug('file ' + relativePath + ' matches \'' + pattern + '\'')
 			return true
+		} else {
+			allPatterns += pattern + ','
 		}
 	}
+	if (allPatterns.length > 0) {
+		log.debug('file ' + relativePath + ' does not match \'' + allPatterns.replace(/,$/, "") + '\'')


doesFileMatch now builds allPatterns via string concatenation inside the loop and then trims a trailing comma. Since this is called frequently during workspace scans, consider collecting patterns and logging patterns.map(p => p.pattern).join(',') once after the loop when no match is found.

Suggested change

let allPatterns = ''

for (const pattern of patterns.map(pattern => pattern.pattern)) {

if (minimatch(relativePath, pattern, { magicalBraces: true })) {

log.debug('file ' + relativePath + ' matches \'' + pattern + '\'')

return true

} else {

allPatterns += pattern + ','

}

}

if (allPatterns.length > 0) {

log.debug('file ' + relativePath + ' does not match \'' + allPatterns.replace(/,$/, "") + '\'')

const patternStrings = patterns.map(pattern => pattern.pattern)

for (const pattern of patternStrings) {

if (minimatch(relativePath, pattern, { magicalBraces: true })) {

log.debug('file ' + relativePath + ' matches \'' + pattern + '\'')

return true

}

}

if (patternStrings.length > 0) {

log.debug('file ' + relativePath + ' does not match \'' + patternStrings.join(',') + '\'')

kenherring

@PeterJudgeZA sorry for all the noise on this PR. This repo hasn't had many forked PRs so figuring out how to get the builds right was a little messy.

There are two things needed to get this PR merged:

Code coverage
Use a map or array or similar data structure rather than a comma delimited list

Concept looks good to me. Really appreciate the pull request and sorry again for all the noise I made trying to get actions to cooperate.

sonarqubecloud · 2026-04-04T06:11:11Z

Quality Gate failed

Failed conditions
50.0% Coverage on New Code (required ≥ 70%)

See analysis details on SonarQube Cloud

kenherring linked an issue Mar 21, 2026 that may be closed by this pull request

🐛 [Bug] Tests not found #448

Open

kenherring mentioned this pull request Mar 21, 2026

Improve calculation of buildPath include and exclude paths #514

Closed

kenherring force-pushed the issue448 branch 10 times, most recently from 109837d to c8caa05 Compare March 29, 2026 01:03

kenherring requested review from Copilot and kenherring March 29, 2026 01:03

Copilot started reviewing on behalf of kenherring March 29, 2026 01:04 View session

Copilot AI reviewed Mar 29, 2026

View reviewed changes

kenherring force-pushed the issue448 branch 3 times, most recently from 175f844 to 60cca31 Compare April 2, 2026 08:31

kenherring requested changes Apr 2, 2026

View reviewed changes

Improve calculation of buildPath include and exclude paths

746db9b

kenherring force-pushed the issue448 branch from 60cca31 to 746db9b Compare April 4, 2026 06:00

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Improve calculation of buildPath include and exclude paths#469

Improve calculation of buildPath include and exclude paths#469
PeterJudgeZA wants to merge 1 commit into
kenherring:mainfrom
PeterJudgeZA:issue448

PeterJudgeZA commented Jan 3, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Mar 29, 2026

Uh oh!

Copilot AI Mar 29, 2026

Uh oh!

Copilot AI Mar 29, 2026

Uh oh!

Copilot AI Mar 29, 2026

Uh oh!

Copilot AI Mar 29, 2026

Uh oh!

Copilot AI Mar 29, 2026

Uh oh!

kenherring left a comment

Uh oh!

sonarqubecloud Bot commented Apr 4, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

		@@ -0,0 +1,41 @@
		import { getDLC, getOEVersion, getOpenEdgeProfileConfig, getBuildPathPatterns } from 'parse/OpenedgeProjectParser'

	import { getDLC, getOEVersion, getOpenEdgeProfileConfig, getBuildPathPatterns } from 'parse/OpenedgeProjectParser'
	import { getOpenEdgeProfileConfig, getBuildPathPatterns } from 'parse/OpenedgeProjectParser'

+suiteTeardown('suiteTeardown', () => {
+	log.group.end()
+})

		for (const b of config.buildPath.filter(b => b.type == 'source') ?? []) {
		const buildPathPatterns = getBuildPathPatterns(workspaceFolder, b)

Conversation

PeterJudgeZA commented Jan 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Mar 29, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 29, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 29, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 29, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 29, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 29, 2026

Choose a reason for hiding this comment

Uh oh!

kenherring left a comment

Choose a reason for hiding this comment

Uh oh!

sonarqubecloud Bot commented Apr 4, 2026

Quality Gate failed

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

PeterJudgeZA commented Jan 3, 2026 •

edited

Loading