Skip to content
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

Move test utilities to the query pack. #17968

Merged
merged 24 commits into from
Dec 16, 2024

Conversation

michaelnebel
Copy link
Contributor

@michaelnebel michaelnebel commented Nov 12, 2024

In this PR the test utilities are moved from the test QL packs into the codeql/<language>-queries QL packs.
We are in the process of moving the experimental queries from the Code QL repo to the CodeQL-Community-Packs repo, and some of the tests for the experimental queries are relying on the test utilities, which currently aren't being published.

As a temporary workaround a copy of the test utilities are added to the CodeQL-Community-Pack, but we should publish the test utilities to enable sharing.

Moving the test utilities for all languages for consistency.

@michaelnebel michaelnebel changed the title Java: Move test utilities to the query pack. Java/C#: Move test utilities to the query pack. Nov 13, 2024
@michaelnebel michaelnebel added the no-change-note-required This PR does not need a change note label Nov 13, 2024
@michaelnebel michaelnebel marked this pull request as ready for review November 13, 2024 10:11
@michaelnebel michaelnebel requested review from a team as code owners November 13, 2024 10:11
@michaelnebel
Copy link
Contributor Author

DCA looks good.

@owen-mc
Copy link
Contributor

owen-mc commented Dec 9, 2024

Should this be done for Go as well? (Not necessarily in this PR.)

@michaelnebel
Copy link
Contributor Author

Should this be done for Go as well? (Not necessarily in this PR.)

Yes, we should do it for all languages, if we agree that this is an acceptable approach. I intend to do it for all languages in this PR, if there are no objections :-)

hvitved
hvitved previously approved these changes Dec 11, 2024
Copy link
Contributor

@hvitved hvitved left a comment

Choose a reason for hiding this comment

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

LGTM

@michaelnebel
Copy link
Contributor Author

Putting in draft while making the corresponding changes for the other languages.

@michaelnebel michaelnebel marked this pull request as draft December 11, 2024 10:36
@michaelnebel michaelnebel changed the title Java/C#: Move test utilities to the query pack. Move test utilities to the query pack. Dec 11, 2024
@michaelnebel michaelnebel marked this pull request as ready for review December 11, 2024 15:00
@michaelnebel michaelnebel requested review from a team as code owners December 11, 2024 15:00
@michaelnebel
Copy link
Contributor Author

michaelnebel commented Dec 12, 2024

Setting ready for review again.

  • All the modules including the post-processing queries have been moved to the language packs.
  • Rust test utilities have been moved ad well.
  • Ignoring the QL doc warnings.
  • The failing C++ integration test looks unrelated.

@michaelnebel michaelnebel marked this pull request as ready for review December 12, 2024 15:40
Copy link
Contributor

@geoffw0 geoffw0 left a comment

Choose a reason for hiding this comment

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

CPP, Swift, Rust LGTM.

@geoffw0
Copy link
Contributor

geoffw0 commented Dec 13, 2024

The failing C++ integration test looks unrelated.

Yes it's an issue that came up yesterday afternoon. I believe it's fixed now on latest semmle-code.

Copy link
Contributor

@egregius313 egregius313 left a comment

Choose a reason for hiding this comment

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

Static languages LGTM

Copy link
Member

@RasmusWL RasmusWL left a comment

Choose a reason for hiding this comment

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

Python 👍

@michaelnebel michaelnebel merged commit aaf0cd5 into github:main Dec 16, 2024
42 of 45 checks passed
@michaelnebel michaelnebel deleted the java/movetestutils branch December 16, 2024 12:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants