Add Windows integration tests showing that `subst` is handled inconsistently by jketema · Pull Request #22087 · github/codeql

jketema · 2026-06-29T15:04:46Z

This shows that we do not consistently handle this across the CLI and the various extractors. All tests are: extract a file, check if the extracted file is relative to the source root, where the source root is a subst drive.

Copilot

Pull request overview

Adds new Windows-only integration tests across multiple language packs to demonstrate inconsistent handling of subst-mapped source roots when extracting files and evaluating whether file paths are treated as relative to the source root.

Changes:

Introduce new subst integration-test directories for Rust, Ruby, Python, JavaScript, Go, Java/Kotlin, and C#.
For each test: create a database with source_root on a subst drive, then run a small query and compare .expected results to show (in)consistencies.
Add minimal source files per language to drive extraction on Windows.

Show a summary per file

File	Description
rust/ql/integration-tests/subst/test.py	Windows integration test that creates a Rust DB with a `subst`-drive source root.
rust/ql/integration-tests/subst/file.ql	Query selecting files and whether a relative path exists (test oracle).
rust/ql/integration-tests/subst/file.expected	Expected results for Rust extraction under `subst`.
rust/ql/integration-tests/subst/code/test.rs	Minimal Rust source for extraction.
ruby/ql/integration-tests/subst/test.py	Windows integration test that creates a Ruby DB with a `subst`-drive source root.
ruby/ql/integration-tests/subst/file.ql	Query selecting files and whether a relative path exists (test oracle).
ruby/ql/integration-tests/subst/file.expected	Expected results for Ruby extraction under `subst`.
ruby/ql/integration-tests/subst/code/test.rb	Minimal Ruby source for extraction.
python/ql/integration-tests/subst/test.py	Windows integration test that creates a Python DB with a `subst`-drive source root.
python/ql/integration-tests/subst/file.ql	Query selecting files and whether a relative path exists (test oracle).
python/ql/integration-tests/subst/file.expected	Expected results for Python extraction under `subst`.
python/ql/integration-tests/subst/code/main.py	Minimal Python source for extraction.
javascript/ql/integration-tests/subst/test.py	Windows integration test that creates a JavaScript DB with a `subst`-drive source root.
javascript/ql/integration-tests/subst/file.ql	Query selecting files and whether a relative path exists (test oracle).
javascript/ql/integration-tests/subst/file.expected	Expected results for JavaScript extraction under `subst`.
javascript/ql/integration-tests/subst/code/test.ts	Minimal TypeScript source for extraction.
javascript/ql/integration-tests/subst/code/main.js	Minimal JavaScript source for extraction.
java/ql/integration-tests/java/subst/test.py	Windows integration test that creates a Java/Kotlin DB with a `subst`-drive source root.
java/ql/integration-tests/java/subst/file.ql	Query selecting files and whether a relative path exists (test oracle).
java/ql/integration-tests/java/subst/file.expected	Expected results for Java/Kotlin extraction under `subst`.
java/ql/integration-tests/java/subst/code/test2.kt	Minimal Kotlin source for extraction.
java/ql/integration-tests/java/subst/code/test1.java	Minimal Java source for extraction.
go/ql/integration-tests/subst/test.py	Windows integration test that creates a Go DB with a `subst`-drive source root.
go/ql/integration-tests/subst/file.ql	Query selecting files and whether a relative path exists (test oracle).
go/ql/integration-tests/subst/file.expected	Expected results for Go extraction under `subst`.
go/ql/integration-tests/subst/code/main.go	Minimal Go source for extraction.
csharp/ql/integration-tests/windows/subst/test.py	Windows integration test that creates a C# DB with a `subst`-drive source root.
csharp/ql/integration-tests/windows/subst/file.ql	Query selecting files and whether a relative path exists (test oracle).
csharp/ql/integration-tests/windows/subst/file.expected	Expected results for C# extraction under `subst`.
csharp/ql/integration-tests/windows/subst/code/Program.cs	Minimal C# program used for extraction/build.
csharp/ql/integration-tests/windows/subst/code/global.json	.NET SDK pin for the test project.
csharp/ql/integration-tests/windows/subst/code/dotnet_build.csproj	Minimal .NET project configuration for extraction/build.

Review details

Files reviewed: 32/32 changed files
Comments generated: 1
Review effort level: Low

jketema · 2026-07-01T10:59:17Z

@@ -0,0 +1,4 @@
+class Program


IdrissRio

Suggestion: It might be worth adding addd comment explaining that some of the .expected files intentionally capture the current, inconsistent behavior. Otherwise, when subst handling is eventually unified, a maintainer regenerating the expectations has no indication that the previous values were documenting a known inconsistency rather than the intended behavior.

From the tests I would say that go, java and javascript handle subst correctly, while puthon, ruby, rust and csharp(?) no.
For those I would add a comment in test.py.

Otherwise LGTM 👍

IdrissRio · 2026-07-01T08:11:49Z

@@ -0,0 +1,4 @@
+class test {


Lowercase test hurts a bit 😆
No need to change.

Fixed, since I was changing things anyway.

IdrissRio · 2026-07-01T08:14:44Z

+where
+  not f.getURL().matches("file:///modules/%") and
+  not f.getURL().matches("file:///!unknown-binary-location/kotlin/%") and
+  not f.getURL().matches("%/semmle-code/semmle-code/%") and


Thinking out loud: are we okay with leaking some of the internal structure of our private repository? Could we use a different filter instead?

Let me have a look. Note that we do leak this in other places already, but not in tests.

jketema · 2026-07-01T08:36:47Z

From the tests I would say that go, java and javascript handle subst correctly

Depends a bit on your perspective, but if you consider a subst drive a symbolic link, then those are exactly the ones that are wrong.

IdrissRio · 2026-07-01T08:56:50Z

but if you consider a subst drive a symbolic link, then those are exactly the ones that are wrong.

That makes perfect sense. Then I would approach it the other way around and add a comment to those test files.

jketema · 2026-07-01T08:58:29Z

That makes perfect sense. Then I would approach it the other way around and add a comment to those test files.

The implication in that case is though that every single one is wrong at the moment.

IdrissRio · 2026-07-01T09:17:25Z

I see. Maybe adding a link to the issue (as a comment) would help here instead of writing down what is right or wrong.

…ently

IdrissRio

Thanks for answering all my questions.

github-actions Bot added C# JS Java Python Go Ruby Rust Pull requests that update Rust code labels Jun 29, 2026

jketema force-pushed the subst branch 3 times, most recently from 3c83d27 to 8e05251 Compare June 29, 2026 15:31

jketema changed the title ~~Add Windows integration tests showing that subst is handled inconsistently~~ Add Windows integration tests showing that subst is handled inconsistently Jun 29, 2026

jketema marked this pull request as ready for review June 29, 2026 15:55

Copilot AI review requested due to automatic review settings June 29, 2026 15:55

jketema requested review from a team as code owners June 29, 2026 15:55

Copilot started reviewing on behalf of jketema June 29, 2026 15:55 View session

Copilot AI reviewed Jun 29, 2026

View reviewed changes

IdrissRio previously approved these changes Jul 1, 2026

View reviewed changes

Add Windows integration tests showing that subst is handled inconsist…

daf97f7

…ently

jketema dismissed IdrissRio’s stale review via daf97f7 July 1, 2026 10:51

jketema force-pushed the subst branch from 8e05251 to daf97f7 Compare July 1, 2026 10:51

IdrissRio previously approved these changes Jul 1, 2026

View reviewed changes

Fix expected file

d551ab3

jketema dismissed IdrissRio’s stale review via d551ab3 July 1, 2026 11:24

IdrissRio approved these changes Jul 1, 2026

View reviewed changes

jketema merged commit d664d17 into github:main Jul 1, 2026
73 of 74 checks passed

jketema deleted the subst branch July 1, 2026 12:48

Uh oh!

Conversation

jketema commented Jun 29, 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

Review details

Uh oh!

jketema Jul 1, 2026

Choose a reason for hiding this comment

Uh oh!

IdrissRio left a comment

Choose a reason for hiding this comment

Uh oh!

IdrissRio Jul 1, 2026

Choose a reason for hiding this comment

Uh oh!

jketema Jul 1, 2026

Choose a reason for hiding this comment

Uh oh!

IdrissRio Jul 1, 2026

Choose a reason for hiding this comment

Uh oh!

jketema Jul 1, 2026

Choose a reason for hiding this comment

Uh oh!

jketema Jul 1, 2026

Choose a reason for hiding this comment

Uh oh!

jketema commented Jul 1, 2026

Uh oh!

IdrissRio commented Jul 1, 2026

Uh oh!

jketema commented Jul 1, 2026

Uh oh!

IdrissRio commented Jul 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

IdrissRio left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

jketema commented Jun 29, 2026 •

edited

Loading

IdrissRio commented Jul 1, 2026 •

edited

Loading