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

Closes #70 Add scenario for pages without LCP/ATF #76

Conversation

Khadreal
Copy link
Contributor

@Khadreal Khadreal commented May 8, 2024

Description

Fixes #70

Documentation

Technical documentation

This PR add checks for pages with no LCP/ATF and check the DB to assert the result are as expected.

Type of change

  • Enhancement (non-breaking change which improves an existing functionality).

New dependencies

N/A

Risks

List possible performance & security issues or risks, and explain how they have been mitigated.

Checklists

Feature validation

  • I validated all the Acceptance Criteria. If possible, provide sreenshots or videos.
  • I triggered all changed lines of code at least once without new errors/warnings/notices.
  • I implemented built-in tests to cover the new/changed code.

Code style

  • I wrote self-explanatory code about what it does.
  • I wrote comments to explain why it does it.
  • I named variables and functions explicitely.
  • I did not introduce unecessary complexity.

@Khadreal Khadreal requested a review from jeawhanlee May 9, 2024 08:29
src/features/lcp-beacon-script.feature Show resolved Hide resolved
src/support/steps/lcp-beacon-script.ts Outdated Show resolved Hide resolved
src/support/steps/lcp-beacon-script.ts Outdated Show resolved Hide resolved
src/support/steps/lcp-beacon-script.ts Outdated Show resolved Hide resolved
utils/page-utils.ts Outdated Show resolved Hide resolved
src/support/steps/general.ts Outdated Show resolved Hide resolved
@MathieuLamiot MathieuLamiot requested a review from jeawhanlee May 13, 2024 16:31
.gitignore Outdated Show resolved Hide resolved
src/support/steps/steps.ts Outdated Show resolved Hide resolved
src/support/steps/steps.ts Outdated Show resolved Hide resolved
@Khadreal Khadreal requested a review from jeawhanlee May 14, 2024 13:00
src/features/lcp-beacon-script.feature Outdated Show resolved Hide resolved
src/support/steps/general.ts Outdated Show resolved Hide resolved
src/support/steps/lcp-beacon-script.ts Outdated Show resolved Hide resolved
jeawhanlee
jeawhanlee previously approved these changes May 15, 2024
src/features/lcp-beacon-script.feature Show resolved Hide resolved
src/support/results/expectedResultsDesktop.json Outdated Show resolved Hide resolved
src/support/results/expectedResultsMobile.json Outdated Show resolved Hide resolved
@jeawhanlee jeawhanlee changed the base branch from trunk to enhancement/71-use-result-file-for-lcp-tests May 16, 2024 08:04
@Khadreal Khadreal requested a review from jeawhanlee May 16, 2024 15:03

Scenario: Beacon captures expected images in desktop
When I log out
And I visit the urls for 'desktop'
Then lcp and atf should be as expected for 'desktop'

Scenario: Beacon captures expected images in mobile
Given I install plugin 'https://github.com/wp-media/wp-rocket-e2e-test-helper/blob/main/helper-plugin/force-wp-mobile.zip'
And plugin 'force-wp-mobile' is activated
When I log out
And I visit the urls for 'mobile'
Then lcp and atf should be as expected for 'mobile'
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
Then lcp and atf should be as expected for 'mobile'
And plugin 'force-wp-mobile' is deactivated

When I log out
And I visit the urls for 'mobile'
Then lcp and atf should be as expected for 'mobile'
And plugin 'force-wp-mobile' is deactivated
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
And plugin 'force-wp-mobile' is deactivated
Then lcp and atf should be as expected for 'mobile'

@jeawhanlee jeawhanlee merged commit 18509cf into enhancement/71-use-result-file-for-lcp-tests May 16, 2024
1 check passed
jeawhanlee added a commit that referenced this pull request May 22, 2024
* Closes #71: Use results file instead of API for lcps

* Comply feedback

* Removed unused hook

* Fix bugs + Update tests results

* Updates expected results
Add is_mobile is sql query
Accept array for lcps

* Comply linter

* Comply feedback

* Updated fixture

* Updated hardcoded atf urls to relative (#81)

* Closes #70 Add scenario for pages without LCP/ATF (#76)

* Add scenario for no lcp/atf, add plugin deactivation function #70 :closes:

* fixed lint

* minor fix

* Add step to activate and deactivate mobile plugin

* update gitignore file

* PR modifications

* Add new command function, pr modifications --#70

* minor modifications

* Add beacon check for lcp pages

* deactivate plugin, modify json

* Add missing step

* change steps

* Closes #82: LCP test not failing with no result in db (#84)

* Make tests fail when no result in db

* Fail tests early

* Make log red

---------

Co-authored-by: Michael Lee <[email protected]>
Co-authored-by: Michael Lee <[email protected]>
Co-authored-by: Opeyemi Ibrahim <[email protected]>
jeawhanlee added a commit that referenced this pull request May 29, 2024
…86)

* Closes #71: Use results file instead of API for lcps

* Comply feedback

* Fix bugs + Update tests results

* Updates expected results
Add is_mobile is sql query
Accept array for lcps

* Comply linter

* Comply feedback

* Updated fixture

* Updated hardcoded atf urls to relative (#81)

* Closes #70 Add scenario for pages without LCP/ATF (#76)

* Add scenario for no lcp/atf, add plugin deactivation function #70 :closes:

* fixed lint

* minor fix

* Add step to activate and deactivate mobile plugin

* update gitignore file

* PR modifications

* Add new command function, pr modifications --#70

* minor modifications

* Add beacon check for lcp pages

* deactivate plugin, modify json

* Add missing step

* change steps

* Closes #82: LCP test not failing with no result in db (#84)

* Make tests fail when no result in db

* Fail tests early

* Make log red

* :chore: Add scenario and step for relative lcp image template -- #85

* PR modifications

---------

Co-authored-by: Gael Robin <[email protected]>
Co-authored-by: Michael Lee <[email protected]>
Co-authored-by: Michael Lee <[email protected]>
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.

Add a scenario for LCP/ATF where no LCP nor ATF is found on the page to ensure the result in DB is as expected
2 participants