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 Tests / Optimize Checking Of plan.title Within spec/features/plans/exports_spec.rb #871

Merged
merged 2 commits into from
Aug 29, 2024

Conversation

aaronskiba
Copy link
Collaborator

Fixes #870

Changes proposed in this PR:

  • Fixes the flaky tests within spec/features/plans/exports_spec.rb (see Some Selenium/Features Tests Breaking (Independent of Code Changes) #870)
    • While inspecting these sometimes failing tests, it was found that expect(page.source).to have_text(plan.title) sometimes failed because sometimes page.source == "".
    • Rather than page.source, which returns the entire HTML content of the page, this PR uses page.title, which only returns the contents inside of the <title> tags.
      • page.title does not seem to encounter the unwanted behaviour of returning a blank string. Maybe because it is faster (only returning the title should be faster than returning the entire HTML content via page.source)?
      • Also, the <title> title tags and their contents are part of the entire HTML content. So despite page.source returning a blank string, because page.title is not blank, it follows that the DOM is not blank.

This commit replaces `page.source` with (the hopefully more efficient) `page.title` for verifying the page title. Checking the entire page source was potentially causing slowdowns and leading to the intermittent failing of tests within this file.
@aaronskiba aaronskiba changed the title Fix flaky tests / Optimize Checking Of plan.title Within spec/features/plans/exports_spec.rb Fix Flaky Tests / Optimize Checking Of plan.title Within spec/features/plans/exports_spec.rb Aug 29, 2024
Copy link
Collaborator

@200455939-yashu 200455939-yashu left a comment

Choose a reason for hiding this comment

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

Good work 👏

@aaronskiba aaronskiba merged commit 929c126 into integration Aug 29, 2024
11 checks passed
@aaronskiba aaronskiba deleted the aaron/issues/870 branch August 29, 2024 21:32
@aaronskiba aaronskiba restored the aaron/issues/870 branch September 4, 2024 16:24
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.

2 participants