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

Select no tests if modified-only returns nothing #2761

Merged
merged 2 commits into from
Mar 20, 2024
Merged

Conversation

lukaszachy
Copy link
Collaborator

@lukaszachy lukaszachy commented Mar 13, 2024

Previously it selected all tests. Now it respects that if nothing is modified then nothing is selected.

Fix: #2449

Is this worth of release note?

Pull Request Checklist

  • implement the feature
  • extend the test coverage

@lukaszachy lukaszachy added the step | discover Stuff related to the discover step label Mar 13, 2024
@lukaszachy lukaszachy added this to the 1.32 milestone Mar 13, 2024
Copy link
Collaborator

@thrix thrix left a comment

Choose a reason for hiding this comment

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

LGTM

@psss psss added the ci | full test Pull request is ready for the full test execution label Mar 20, 2024
Copy link
Collaborator

@psss psss left a comment

Choose a reason for hiding this comment

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

Thanks for the fix, looks good. Do you still plan to add a release not as suggested by the checklist?

@psss psss requested a review from happz March 20, 2024 08:47
@psss psss changed the title Select no tests if modified-only returns nothing Select no tests if modified-only returns nothing Mar 20, 2024
@lukaszachy
Copy link
Collaborator Author

Do you still plan to add a release not as suggested by the checklist?

No idea. Any preference? On one side it changes behavior but on the other it changes it to the expected one.

@psss
Copy link
Collaborator

psss commented Mar 20, 2024

On one side it changes behavior but on the other it changes it to the expected one.

Yeah, in fact, it just fixes a bug, and probably not that interesting --> I suggest to skip the release note.

@psss
Copy link
Collaborator

psss commented Mar 20, 2024

Hm, /tests/discover/modified seems to be failing.

@lukaszachy
Copy link
Collaborator Author

I'll take a look in the afternoon. That test used to pass before the rebase.

Previously it selected all tests. Now it respects that if nothing is
modified then nothing is selected.

Fix: #2449
@lukaszachy
Copy link
Collaborator Author

lukaszachy commented Mar 20, 2024

Ah, right. I broke it in fbfd0e0.

Fixed now. Tests are good.

@psss
Copy link
Collaborator

psss commented Mar 20, 2024

Failing tests are known culprits --> merging.

@psss psss self-assigned this Mar 20, 2024
@psss psss merged commit 5131014 into main Mar 20, 2024
18 of 20 checks passed
@psss psss deleted the 2449-modified-only-empty branch March 20, 2024 14:01
@psss psss restored the 2449-modified-only-empty branch March 21, 2024 10:14
@psss psss deleted the 2449-modified-only-empty branch March 21, 2024 10:14
The-Mule pushed a commit to The-Mule/tmt that referenced this pull request Oct 14, 2024
Previously it selected all tests. Now it respects that if nothing is
modified then nothing is selected.

Fix: teemtee#2449
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ci | full test Pull request is ready for the full test execution step | discover Stuff related to the discover step
Projects
None yet
Development

Successfully merging this pull request may close these issues.

discover fmf with modified-only: true runs all the test if no files were modified
5 participants