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

Fix envtool run test -run and -skip flags #4101

Merged
merged 15 commits into from Feb 26, 2024
Merged

Conversation

henvic
Copy link
Contributor

@henvic henvic commented Feb 20, 2024

Description

The logic for filtering the tests should be applied before sharding and use the same logic as the Go standard testing library.

Closes #3969.

Readiness checklist

  • I added/updated unit tests (and they pass).
  • I added/updated integration/compatibility tests (and they pass).
  • I added/updated comments and checked rendering.
  • I made spot refactorings.
  • I updated user documentation.
  • I ran task all, and it passed.
  • I ensured that PR title is good enough for the changelog.
  • (for maintainers only) I set Reviewers (@FerretDB/core), Milestone (Next), Labels, Project and project's Sprint fields.
  • I marked all done items in this checklist.

@mergify mergify bot assigned henvic Feb 20, 2024
Copy link

codecov bot commented Feb 20, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 75.07%. Comparing base (0aad83a) to head (266af4e).

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #4101      +/-   ##
==========================================
- Coverage   76.22%   75.07%   -1.16%     
==========================================
  Files         349      349              
  Lines       22170    22170              
==========================================
- Hits        16899    16644     -255     
- Misses       3960     4258     +298     
+ Partials     1311     1268      -43     

see 12 files with indirect coverage changes

Flag Coverage Δ
filter-true 67.32% <ø> (-1.36%) ⬇️
hana-1 ?
integration 67.32% <ø> (-1.36%) ⬇️
mongodb-1 4.69% <ø> (ø)
postgresql-1 49.05% <ø> (+0.02%) ⬆️
postgresql-2 48.70% <ø> (-0.08%) ⬇️
postgresql-3 48.77% <ø> (+<0.01%) ⬆️
sqlite-1 48.21% <ø> (+0.02%) ⬆️
sqlite-2 47.88% <ø> (+0.01%) ⬆️
sqlite-3 47.92% <ø> (ø)
unit 33.16% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

@henvic henvic added the code/chore Code maintenance improvements label Feb 20, 2024
@henvic henvic added this to the Next milestone Feb 20, 2024
@henvic henvic marked this pull request as ready for review February 20, 2024 09:28
@henvic henvic requested review from a team and AlekSi as code owners February 20, 2024 09:28
@henvic henvic enabled auto-merge (squash) February 20, 2024 09:34
.golangci.yml Outdated Show resolved Hide resolved
rumyantseva
rumyantseva previously approved these changes Feb 23, 2024
Copy link
Member

@rumyantseva rumyantseva left a comment

Choose a reason for hiding this comment

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

I tried the fixes locally, I think it works!

Copy link
Contributor

mergify bot commented Feb 23, 2024

@henvic this pull request has merge conflicts.

@mergify mergify bot added the conflict PRs that have merge conflicts label Feb 23, 2024
@mergify mergify bot removed the conflict PRs that have merge conflicts label Feb 23, 2024
Copy link
Member

@noisersup noisersup left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@chilagrow chilagrow left a comment

Choose a reason for hiding this comment

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

Great work!

@henvic henvic merged commit 81e0be8 into FerretDB:main Feb 26, 2024
35 of 51 checks passed
shard, err := shardTestFuncs(index, total, all)
// Then, shard all the tests but only run the ones that match the regex and that should
// be run on the specific shard.
shard, skipShard, err := shardTestFuncs(index, total, tests)
Copy link
Member

Choose a reason for hiding this comment

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

Note that shard's content is not actually used

}

if skip != "" {
args = append(args, "-skip="+skip)
Copy link
Member

Choose a reason for hiding this comment

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

I don't understand what we are doing there. #3969's title is "Process --run and --skip in the envtool itself". Somehow, we both process those flags ourselves, then ignore the result for the --run processing, but pass -skip.

What I think we should do is what the issue says. In more detail:

  • get --run and --skip values;
  • get a list of all tests;
  • filter that list by applying --run and --skip values;
  • run those tests by invoking go test -run – without -skip because we already process --skip's regular expression.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
code/chore Code maintenance improvements
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

Process --run and --skip in the envtool itself
7 participants