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

e2e: fix e2e by fixing state transitions in %notify #3335

Conversation

patosullivan
Copy link
Member

Fixes LAND-1682 by fixing an issue in the state types in %notify.

At one point, the notification type changed and was subtly wrong. This went unnoticed because we used to drop old state anytime it didn't match the current state type.

This began to fail when we introduced changes to the %notify agent that included state transitions and there were ships that had not yet had that old state dropped.

This particularly effected the fake ships we use for e2e tests, because they boot with an old version of %groups. This would have also effected any ships that had not yet dropped their old state %3 and replaced it with the "new" state %3 with the bad notification type.

We get around this issue by just accepting any noun for the notifications type for base-state-2 and base-state-3, and creating a new base-state-4 with the correct type. Ships on either version of state %3 or already on state %4 should transition without issue now.

Fixes LAND-1682 by fixing an issue in the state types in %notify.

At one point, the `notification` type changed and was subtly wrong. This went unnoticed because we used to drop old state anytime it didn't match the current state type.

This began to fail when we introduced changes to the %notify agent that included state transitions and there were ships that had not yet had that old state dropped.

This particularly effected the fake ships we use for e2e tests, because they boot with an old version of %groups. This would have also effected any ships that had not yet dropped their old state %3 and replaced it with the "new" state %3 with the bad notification type.

We get around this issue by just accepting any noun for the `notifications` type for base-state-2 and base-state-3, and creating a new base-state-4 with the correct type. Ships on either version of state %3 or already on state %4 should transition without issue now.
Copy link

linear bot commented Mar 14, 2024

Copy link
Member

@Fang- Fang- left a comment

Choose a reason for hiding this comment

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

Mother of all branch names.

We paired on this, so of course I would say "LGTM!"

@patosullivan patosullivan merged commit b864b25 into develop Mar 14, 2024
1 check passed
@patosullivan patosullivan deleted the po/land-1682-e2e-fix-commit-hash-diff-issue-by-fixing-notify-state branch March 14, 2024 14:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants