Skip to content

test(e2e): reverting PR3211 since it is making e2e nightly to fail#3335

Merged
andynog merged 1 commit intomainfrom
andy/revert-3211
Jun 24, 2024
Merged

test(e2e): reverting PR3211 since it is making e2e nightly to fail#3335
andynog merged 1 commit intomainfrom
andy/revert-3211

Conversation

@andynog
Copy link
Collaborator

@andynog andynog commented Jun 24, 2024

This PR reverts #3211 since it is making the e2e nightly to fail in reproducible ways. This PR was identified as the reason for the recent e2e nightly failure on main and doing a bi-sect test of all recent commits it was determined that this PR introduced a behavior that makes the tests to fail and indicate a bug or unknown behavior has been introduced.

Even though we are reverting this logic for now, we'd be happy to consider it in the future again once more tests are performed and we can ensure it passes all tests.


PR checklist

  • Tests written/updated
  • Changelog entry added in .changelog (we use unclog to manage our changelog)
  • Updated relevant documentation (docs/ or spec/) and code comments
  • Title follows the Conventional Commits spec

@andynog andynog added e2e Related to our end-to-end tests testing related to unit testing in general labels Jun 24, 2024
@andynog andynog added this to the 2024-Q2 milestone Jun 24, 2024
@andynog andynog self-assigned this Jun 24, 2024
@andynog andynog requested a review from a team as a code owner June 24, 2024 15:23
@andynog andynog requested a review from a team June 24, 2024 15:23
Copy link
Collaborator

@sergio-mena sergio-mena left a comment

Choose a reason for hiding this comment

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

Checked, via git fiddling, that this is the reverse patch. ✅

Copy link

@cason cason left a comment

Choose a reason for hiding this comment

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

Diffs match :)

@andynog andynog added this pull request to the merge queue Jun 24, 2024
Merged via the queue into main with commit e564c70 Jun 24, 2024
@andynog andynog deleted the andy/revert-3211 branch June 24, 2024 16:02
@ValarDragon
Copy link
Contributor

Reading the logs, I think this issue should be easy for us to solve.

It seems that the reactor's view of initial height on some nodes is incorrectly being set to 0 (rather than the true initial height). I think theres only two causes for this:

  • updateToState hasn't been called yet
  • (unlikely) initialHeight hasn't propogated to state yet. PR'ing a fix.

ValarDragon added a commit that referenced this pull request Jun 26, 2024
@cason
Copy link

cason commented Jun 26, 2024

So lets revert the reversion!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

e2e Related to our end-to-end tests testing related to unit testing in general

Projects

No open projects
Status: Done

Development

Successfully merging this pull request may close these issues.

4 participants