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

mark flaky tests #3319

Merged
merged 1 commit into from
Jun 28, 2023
Merged

Conversation

farodin91
Copy link
Contributor

@farodin91 farodin91 commented Nov 11, 2022


Thank you for contributing to JanusGraph!

In order to streamline the review of the contribution we ask you
to ensure the following steps have been taken:

For all changes:

  • Is there an issue associated with this PR? Is it referenced in the commit message?
  • Does your PR body contain #xyz where xyz is the issue number you are trying to resolve?
  • Has your PR been rebased against the latest commit within the target branch (typically master)?
  • Is your initial contribution a single, squashed commit?

For code changes:

  • Have you written and/or updated unit tests to verify your changes?
  • If adding new dependencies to the code, are these dependencies licensed in a way that is compatible for inclusion under ASF 2.0?
  • If applicable, have you updated the LICENSE.txt file, including the main LICENSE.txt file in the root of this repository?
  • If applicable, have you updated the NOTICE.txt file, including the main NOTICE.txt file found in the root of this repository?

For documentation related changes:

  • Have you ensured that format looks appropriate for the output in which it is rendered?

@FlorianHockmann
Copy link
Member

Thanks for working on this! I had to manually restart jobs quite often lately due to flaky tests that aren't marked yet (but have an open issue).

@rngcntr
Copy link
Contributor

rngcntr commented Dec 14, 2022

[deleted]

@rngcntr
Copy link
Contributor

rngcntr commented Dec 14, 2022

It surprises me that so many checks have failed for this PR. The logs are expired, can you re-run the tests?
Also, is there a reason this is still marked as a draft?

@FlorianHockmann
Copy link
Member

Fix #3096
Fix #3142
Fix #3132
Fix #3352

This PR only marks the flaky tests for JUnit to execute them multiple times so we don't have to do that manually, but we should still fix the tests to be reliable so we don't need that at all. So, the issues should stay open until we have improved the tests themselves. See also the description in TESTING.md

@FlorianHockmann
Copy link
Member

I just rebased this on master to fix merge conflicts and to see why some jobs were failing.

@farodin91: I'd like to get this merged soon as the flaky tests are really a pain. I have to restart jobs all the time which especially slows down merging of Dependabot PRs.

Signed-off-by: Jan Jansen <[email protected]>
@FlorianHockmann FlorianHockmann marked this pull request as ready for review June 21, 2023 15:01
Copy link
Member

@FlorianHockmann FlorianHockmann left a comment

Choose a reason for hiding this comment

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

This is now ready for review. The changes were done by @farodin91. I only updated the branch and removed the annotation of one test which doesn't look flaky to me and where the annotation apparently didn't really work because the test isn't suitable to be executed repeatedly.

@FlorianHockmann FlorianHockmann added this to the Release v1.0.0 milestone Jun 28, 2023
@FlorianHockmann
Copy link
Member

Merging via lazy consensus

@FlorianHockmann FlorianHockmann merged commit 2e54144 into JanusGraph:master Jun 28, 2023
@FlorianHockmann FlorianHockmann deleted the mark-flaky-tests branch June 28, 2023 13:45
@janusgraph-automations
Copy link

💚 All backports created successfully

Status Branch Result
v0.6

Note: Successful backport PRs will be merged automatically after passing CI.

Questions ?

Please refer to the Backport tool documentation and see the Github Action logs for details

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.

5 participants