Skip to content

Add a CLI test for MRVA with multiple queries#3354

Merged
robertbrignull merged 4 commits intomainfrom
robertbrignull/multi-mrva-test
Feb 14, 2024
Merged

Add a CLI test for MRVA with multiple queries#3354
robertbrignull merged 4 commits intomainfrom
robertbrignull/multi-mrva-test

Conversation

@robertbrignull
Copy link
Copy Markdown
Contributor

This PR adds a simple test for running MRVA with two queries. This was actually quite easy because we can re-use the doVariantAnalysisTest method with only very small changes.

There are more edge cases of course, but I think they're mostly covered by the no-workspace integration tests. In the CLI tests which are even slower to run, I hope it's ok to just run a single MRVA with two queries and that shows that the process works.

There is already a test for running queries from a published pack, so we don't need to add anything more for that.

Checklist

  • CHANGELOG.md has been updated to incorporate all user visible changes made by this pull request.
  • Issues have been created for any UI or other user-facing changes made by this pull request.
  • [Maintainers only] If this pull request makes user-facing changes that require documentation changes, open a corresponding docs pull request in the github/codeql repo and add the ready-for-doc-review label there.

@robertbrignull robertbrignull requested a review from a team as a code owner February 13, 2024 17:50
@charisk
Copy link
Copy Markdown
Contributor

charisk commented Feb 14, 2024

Generally looking good. Let's make sure the tests pass in the various versions of the CLI that we expect them too. Would be good to run the CLI tests workflow against your branch.

@robertbrignull
Copy link
Copy Markdown
Contributor Author

Let's make sure the tests pass in the various versions of the CLI that we expect them too. Would be good to run the CLI tests workflow against your branch.

Good point the test should only run on supported CLI versions.

I'm a bit confused why it's currently failing on the latest CLI version as it passed locally. I'll take a look.

@robertbrignull robertbrignull requested a review from a team as a code owner February 14, 2024 10:07
@robertbrignull
Copy link
Copy Markdown
Contributor Author

robertbrignull commented Feb 14, 2024

Run of all CLI tests: https://github.com/github/vscode-codeql/actions/runs/7900721664 (updated)

@robertbrignull robertbrignull force-pushed the robertbrignull/multi-mrva-test branch from 02b34cc to 2138f85 Compare February 14, 2024 11:49
@robertbrignull
Copy link
Copy Markdown
Contributor Author

The CLI tests are all green. Should be ready for re-review now.

Copy link
Copy Markdown
Contributor

@charisk charisk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚢

@robertbrignull robertbrignull merged commit 9f4b827 into main Feb 14, 2024
@robertbrignull robertbrignull deleted the robertbrignull/multi-mrva-test branch February 14, 2024 12:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants