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 specs in backend/spec/features/admin/users_spec.rb #3089

Merged
merged 2 commits into from
Feb 20, 2019

Conversation

spaghetticode
Copy link
Member

(This is a reopening for PR #3061 closed by mistake)

These specs are failing often on circleCI builds.

The added expectation ensures that the click toggled the table header HTML.
This has the double benefit to ensure HTML is consistent and leaves more time
to successfully verify the flaky expectation:

expect(text_match_1).to appear_before text_match_2

Also, the within_table in the second example must be splitted in order to get
green specs. This is seems to be caused by rack_test not being able to detect
updates inside the selected HTML block after a page reload.

Re-selecting the HTML block with a second within_table does the job.

Checklist:

  • I have followed Pull Request guidelines
  • I have added a detailed description into each commit message
  • I have updated Guides and README accordingly to this change (if needed)
  • I have added tests to cover this change (if needed)
  • I have added a CHANGELOG entry for this change (if needed)

These specs are failing often on circleCI builds.

The added expectation ensures that the click toggled the table header HTML.
This has the double benefit to ensure HTML is consistent and leaves more time
to successfully verify the flaky expectation:

`expect(text_match_1).to appear_before text_match_2`
The `within_table` must be splitted in order to get green specs. This is seems
be caused by rack_test not being able to detect updates inside the selected
HTML block after a page reload.

Re-selecting the HTML block with a second `within_table` does the job.
@kennyadsl
Copy link
Member

@spaghetticode maybe we should change the first commit message? Or are we still uncertain it will fix?

@spaghetticode
Copy link
Member Author

@kennyadsl I'm still kind of uncertain.

I mean, there was no real reason in the first place for these specs to be broken, in fact most of the time they were green anyway... apart from the real issue I fixed with the second commit (which anyway was not relevant for the original appear_before failing check, as appear_before uses the unscoped page.body) I could not find something deterministic in the failures.

I think only the future can make us certain about this.

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.

@kennyadsl
Copy link
Member

@solidusio/core-team can we have another review here, please?

Copy link
Member

@tvdeyen tvdeyen left a comment

Choose a reason for hiding this comment

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

Thanks!

@kennyadsl kennyadsl merged commit 96c86a6 into solidusio:master Feb 20, 2019
@kennyadsl kennyadsl deleted the spaghetticode/flaky-spec branch February 20, 2019 22:01
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.

3 participants