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

Retry new or modified tests based on git diff #102

Closed

Conversation

wyhasany
Copy link

The best time to resolve flaky test is once you wrote that test. This is
how Google looks for flakes in their monorepo. At that moment you have
the best knowledge to solve such tests. Otherwise after month of running
such test the cognitive load to fix it is very high.

This PR adds a way to discover all changed test classes with diff to
master branch and retry them until the maxRetry or to the first fail
execution.

Unfortunately it breaks configuration cache support. Could you help me
to solve that issue?

@wyhasany
Copy link
Author

@ldaley / @breskeby could you take a look to my changes. Do you have any suggestions?

@wyhasany wyhasany force-pushed the feature/retry-modified-or-new-tests branch from d6ecb3b to 0823eaa Compare May 26, 2021 20:09
The best time to resolve flaky test is once you wrote that test. This is
how Google looks for flakes in their monorepo. At that moment you have
the best knowledge to solve such tests. Otherwise after month of running
such test the cognitive load to fix it is very high.

This PR adds a way to discover all changed test classes with diff to
master branch and retry them until the maxRetry or to the first fail
execution.

Signed-off-by: Michał Rowicki <[email protected]>
@wyhasany wyhasany force-pushed the feature/retry-modified-or-new-tests branch from 0823eaa to 7f1c5ff Compare May 26, 2021 20:11
@ldaley
Copy link
Member

ldaley commented Jun 1, 2021

Hi @wyhasany, thanks for the great contribution.

I don't think this plugin is the right place to embed this functionality. It would be a better separation of concerns to have a separate plugin that adds the functionality you are adding here, and have it collaborate with this plugin.

That's at least theoretically possible by having your plugin populate the filter.includedClasses of the extension, based on your inspection of the history.

@wyhasany
Copy link
Author

wyhasany commented Jun 1, 2021

Hi @ldaley thanks for your feedback. Yes that's possible to populate filter.includedClasses by another plugin (I suppose to). But it won't work then as I suppose. Correct me if I'm wrong, the filter.includedClasses wouldn't be repeated up to maxRetry by default now. The purpose of my plugin is to repeat all the new modified tests despite the first success result up to the maxRetry value or first fail. It allows us to discover flaky on the very early stage.

The idea of that PR comes from this Google presentation:
https://youtu.be/FrBN94gUn_I?t=1759

Could you elaborate with more details what is another way to implement proposed PR? I'm open for ideas 👍

@ldaley
Copy link
Member

ldaley commented Jun 8, 2021

@wyhasany I originally missed that this was based retry-until-fail, not retry-until-success. In that case, let's add that functionality generically. Then a separate, Git specific, plugin can build on that.

There are a few things to sort out:

  • What is the user facing API on TestRetryExtension for nominating which classes are subject to retry-until-fail?
  • How exactly does it behave when there are some failures, but some retry-until-failure test executions are pending? And with --fail-fast?
  • If a user has indicated that a test is subject to retry-until-success and retry-until-failure, what happens?

Probably more too, but let's start with that.

@dblock
Copy link

dblock commented Nov 15, 2022

Coming from opensearch-project/OpenSearch#5226 this is a great idea!

@pshevche
Copy link
Member

@wyhasany , we have recently discussed internally the future of retry-until-success feature in this plugin, which is commonly requested. We decided to keep the plugin focused on retrying failures, and recommend using test framework features to achieve a similar behaviour (e.g., Spock's RepeatUntilFailure extension).

Retrying based on git diff sounds like a great idea, but as Luke suggested, it might make sense to add it as a separate plugin. I'll close this PR for now, but feel free to open an issue if there is anything we can assist with (for example, provide an integration point for the diff-based retries).

@pshevche pshevche closed this Apr 18, 2024
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.

4 participants