Skip to content

Move tests out of pure directory#2541

Merged
robertbrignull merged 1 commit intomainfrom
robertbrignull/move-pure-4
Jun 22, 2023
Merged

Move tests out of pure directory#2541
robertbrignull merged 1 commit intomainfrom
robertbrignull/move-pure-4

Conversation

@robertbrignull
Copy link
Copy Markdown
Contributor

When moving code out of the /pure directory and into /common and other directories, I forgot about the tests. This PR moves all of the tests out of /pure and into other directories to match the files they are testing.

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 June 22, 2023 14:21
@robertbrignull robertbrignull requested a review from a team as a code owner June 22, 2023 14:21
Copy link
Copy Markdown
Contributor

@shati-patel shati-patel left a comment

Choose a reason for hiding this comment

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

Good spot! 🪝

Out of interest, did the "Unwanted dependency on vscode API" CodeQL query catch these test files? Should it?

@robertbrignull
Copy link
Copy Markdown
Contributor Author

Out of interest, did the "Unwanted dependency on vscode API" CodeQL query catch these test files? Should it?

That wasn't how I noticed about the test files and I don't think we'd expect it to alert on any of the tests files. In the query we only look for files in the "src" directory.

We could expand the query to include the tests, but it's a little less important, and the tests themselves will fail if you include a vscode dependency in the unit tests. I think it's fine as it is and we don't need to adjust the query.

@robertbrignull robertbrignull enabled auto-merge June 22, 2023 15:53
@robertbrignull robertbrignull merged commit fa23441 into main Jun 22, 2023
@robertbrignull robertbrignull deleted the robertbrignull/move-pure-4 branch June 22, 2023 16:09
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.

2 participants