-
Notifications
You must be signed in to change notification settings - Fork 1.8k
Rust: Split up the dataflow/sources test #20673
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Presumably because of a change in context, e.g. the module wrapping this test now, or the different inputs. Either way we *should* be able to get the result, and what we *actually* get in our analysis doesn't change here either. We've just slightly changed the test and exposed a gap.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR reorganizes the dataflow/sources test suite by splitting a large monolithic test into smaller, more focused test suites organized by source type. This improves local development workflow by reducing dependencies and compilation times.
Key changes:
- Split the original
dataflow/sourcestest into multiple focused test directories - Created separate test suites for: web frameworks, stdin, networking, file I/O, environment variables, and database sources
- Maintained test coverage while improving modularity
Reviewed Changes
Copilot reviewed 44 out of 54 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
web_frameworks/test.rs |
(Implied) Test cases for web framework data sources |
web_frameworks/*.qlref |
Query references for web framework taint source tests |
web_frameworks/*.expected |
Expected test results for web framework sources |
stdin/test.rs |
Test cases for standard input data sources |
stdin/*.qlref |
Query references for stdin taint source tests |
stdin/*.expected |
Expected test results for stdin sources |
net/test.rs |
Test cases for network-related data sources (HTTP, TCP) |
net/options.yml |
Reduced dependency list, removed web framework dependencies |
net/*.qlref |
Query references for network taint source tests |
net/*.expected |
Expected test results for network sources |
file/test.rs |
Test cases for file system data sources |
file/options.yml |
Dependencies for file I/O tests |
file/*.qlref |
Query references for file taint source tests |
env/test.rs |
Test cases for environment variable and command-line argument sources |
env/options.yml |
Minimal dependencies for environment tests |
env/*.qlref |
Query references for environment taint source tests |
database/test.rs |
Test cases for database query result sources |
database/options.yml |
Dependencies for MySQL database tests |
database/*.qlref |
Query references for database taint source tests |
test.rs |
(Deleted) Original monolithic test file |
test_futures_io.rs |
(Deleted) Moved to net directory |
reqwest.rs |
(Deleted) Stub file removed |
| @@ -0,0 +1,2 @@ | |||
| query: ../env/InlineFlow.ql | |||
Check warning
Code scanning / CodeQL
Query test without inline test expectations Warning test
| @@ -0,0 +1,2 @@ | |||
| query: ../env/InlineFlow.ql | |||
Check warning
Code scanning / CodeQL
Query test without inline test expectations Warning test
| @@ -0,0 +1,2 @@ | |||
| query: ../env/InlineFlow.ql | |||
Check warning
Code scanning / CodeQL
Query test without inline test expectations Warning test
| @@ -0,0 +1,2 @@ | |||
| query: ../env/InlineFlow.ql | |||
Check warning
Code scanning / CodeQL
Query test without inline test expectations Warning test
| @@ -0,0 +1,2 @@ | |||
| query: ../env/InlineFlow.ql | |||
Check warning
Code scanning / CodeQL
Query test without inline test expectations Warning test
|
Can we please wait until #20282 is merged? I have already had to rebase that PR multiple times because of exactly this test. |
Yes, if we get that merged soon. Fixing merge conflicts here is going to be a bit of a pain, I tried to pick a quiet time for it. |
Has now been merged. |
|
I've merged in main (with #20282). Had to do a bit of manual fixup. There had been two results lost in this PR (I hypothesised because of something about their context changing), we get them back now! :) I would appreciate review and approval of this work as soon as possible, assuming there are no big objections. Otherwise difficult merge conflicts are going to keep happening due to the nature of this change. |
paldepind
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for doing this 👍
Split up the
dataflow/sourcestest. It had become quite large, with lots of dependencies, which was making it slow to work with locally.