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

Improve test quality by either improving or removing unreliable test #692

Closed
Jeffwan opened this issue Nov 5, 2022 · 4 comments · Fixed by #1341
Closed

Improve test quality by either improving or removing unreliable test #692

Jeffwan opened this issue Nov 5, 2022 · 4 comments · Fixed by #1341
Labels
P1 Issue that should be fixed within a few weeks

Comments

@Jeffwan
Copy link
Collaborator

Jeffwan commented Nov 5, 2022

image

This actually looks really bad. We don't even know it's flaky tests or test failures.

  1. Could maintainers make sure tests pass before merging the PR?
  2. Could maintainers rerun the test for post-submit commits?
  3. is there a way to open rerun permission for PR author?
@kevin85421
Copy link
Member

kevin85421 commented Nov 5, 2022

Every PR will pass all tests before merging. There are a lot of questions (e.g. #618 #634 #638) in the existing compatibility tests. Some of them are fixed, e.g. #620 and #621. We still need more time to fix all of them.

Is there a way to open rerun permission for PR author?

This is a good idea. We can discuss this in the next community sync.

@Jeffwan
Copy link
Collaborator Author

Jeffwan commented Nov 8, 2022

For short term, I think it's better to rerun the tests for merged commits before the compatibility get fixed

@Jeffwan
Copy link
Collaborator Author

Jeffwan commented Nov 8, 2022

image

@DmitriGekhtman
Copy link
Collaborator

DmitriGekhtman commented Nov 9, 2022

For short term, I think it's better to rerun the tests for merged commits before the compatibility get fixed

Makes sense!

Some other potential actions we considered:
Isolate flaky tests.
Move Fault Tolerance tests to a nightly pipeline.
Consider modifying test timeouts.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P1 Issue that should be fixed within a few weeks
Projects
None yet
3 participants