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

Fix flaky product feature spec #4118

Merged

Conversation

gsmendoza
Copy link
Contributor

@gsmendoza gsmendoza commented Jun 24, 2021

Description

Fixes #4110.

Compared to #4114, this PR separates the refactoring part of the fix (inlining the root_path visit) from the actual fix. The PR also individually improves the other specs affected by the refactoring, documenting in the commits why the improvements were safe to make.

Video walk-through of the PR

https://www.dropbox.com/s/jo4fctehbdhsmix/4118-walkthough-2021-06-25_08.28.36.mp4?dl=0

Relevant links

Checklist

George Mendoza added 6 commits June 25, 2021 08:35
Fixes solidusio#4110.

Based on the spec description, it's unlikely that the spec intended to
visit the root path twice, while setting up the variants in between
visits.

Analysis of the cause of the flaky spec can be found at
solidusio#4110 (comment).
It doesn't appear that the visit is relevant to the Russian Rubles
specs.
It doesn't appear that the spec is intentionally visiting the page
before setting up the fixtures.
@gsmendoza gsmendoza force-pushed the 4110-flaky-feature-spec-detected-fix branch from ec7a554 to 4161874 Compare June 25, 2021 00:36
@gsmendoza gsmendoza marked this pull request as ready for review June 25, 2021 00:39
Copy link
Member

@kennyadsl kennyadsl left a comment

Choose a reason for hiding this comment

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

Thanks!

@spaghetticode spaghetticode merged commit b9938c3 into solidusio:master Jul 1, 2021
@spaghetticode spaghetticode deleted the 4110-flaky-feature-spec-detected-fix branch July 1, 2021 05:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Flaky Feature Spec detected
4 participants