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.

Sign up for GitHub

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

Jump to bottom

--no-healthcheck, no shorthands and test necromancy #4184

Merged
merged 2 commits into from
Apr 12, 2024
Merged

--no-healthcheck, no shorthands and test necromancy #4184

merged 2 commits into from
Apr 12, 2024

Conversation

LesnyRumcajs
Copy link
Member

@LesnyRumcajs LesnyRumcajs commented Apr 11, 2024 •
edited

Summary of changes

Changes introduced in this pull request:

  • re-enabled importing_bad_snapshot_should_fail - I was not able to reproduce the failure, if it happens again let's investigate it.
  • simplified common conditions for tests, so that no servers are started by default (might help with some flakiness)
  • added a flag to disable healthcheck
  • ⚠️ Breaking change. Removed shorthand flags from forest. They were not helpful; someone seeing forest -n might have been surprised it runs Forest with disabled metrics.

Reference issue to close (if applicable)

Closes #3449

Other information and links

Change checklist

  • I have performed a self-review of my own code,
  • I have made corresponding changes to the documentation,
  • I have added tests that prove my fix is effective or that my feature works (if possible),
  • I have made sure the CHANGELOG is up-to-date. All user-facing changes should be reflected in this document.

@LesnyRumcajs LesnyRumcajs force-pushed the repro-flaky-test branch 3 times, most recently from 8a4ff78 to 0ae2808 Compare April 11, 2024 16:14
@LesnyRumcajs LesnyRumcajs force-pushed the repro-flaky-test branch 2 times, most recently from 722d745 to 1ae46ff Compare April 11, 2024 17:05
@LesnyRumcajs LesnyRumcajs changed the title reproduce flaky test --no-healthcheck, no shorthands and test necromancy Apr 11, 2024
@LesnyRumcajs LesnyRumcajs marked this pull request as ready for review April 11, 2024 17:13
@LesnyRumcajs LesnyRumcajs requested a review from a team as a code owner April 11, 2024 17:13
@LesnyRumcajs LesnyRumcajs requested review from ruseinov and lemmih and removed request for a team April 11, 2024 17:13
hanabi1224
hanabi1224 reviewed Apr 12, 2024
View reviewed changes
tests/import_snapshot_tests.rs
#[test]
#[ignore]
fn importing_bad_snapshot_should_fail() {
Copy link
Contributor

Choose a reason for hiding this comment

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

Nice to see it's summoned by our necromancer : )

@LesnyRumcajs LesnyRumcajs added this pull request to the merge queue Apr 12, 2024
Merged via the queue into main with commit ccfcf4d Apr 12, 2024
30 checks passed
@LesnyRumcajs LesnyRumcajs deleted the repro-flaky-test branch April 12, 2024 06:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lemmih lemmih lemmih approved these changes

@hanabi1224 hanabi1224 hanabi1224 approved these changes

@ruseinov ruseinov Awaiting requested review from ruseinov ruseinov is a code owner automatically assigned from ChainSafe/forest

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

importing_bad_snapshot_should_fail test is timing out intermittently
3 participants
@LesnyRumcajs @lemmih @hanabi1224