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

Enhancement/refactor smoke test #142

Merged
merged 18 commits into from
Sep 19, 2024
Merged

Conversation

Mai-Saad
Copy link
Contributor

@Mai-Saad Mai-Saad commented Sep 17, 2024

Description

This PR attempts to solve smoke test errors related to this.page.waitForLoadState('load', { timeout: 30000 }); and refactor where needed for logic

Note: still some tests are failing so it's WIP

Fixes #133 partially

Type of change

  • New feature (non-breaking change which adds functionality).
  • Bug fix (non-breaking change which fixes an issue).
  • Enhancement (non-breaking change which improves an existing functionality).
  • Breaking change (fix or feature that would cause existing functionality to not work as before).
  • Sub-task of #(issue number)
  • Release

Detailed scenario

Sometimes tests are failing as this.page.waitForLoadState('load', { timeout: 30000 }); timeout so here we replaced some of waitforload with assertion + some logic refactor

Technical description

Documentation

  • It replaces timeout:30000 in some places with an assertion and decreases wait time in other parts
  • It refactors logic in some parts to remove unneeded steps

New dependencies

No

Risks

No

Mandatory Checklist

Code validation

  • I triggered all changed lines of code at least once without new errors/warnings/notices.

Code style

  • I wrote a self-explanatory code about what it does.
  • I did not introduce unnecessary complexity.

Additional Checks

  • In the case of complex code, I wrote comments to explain it.

@Mai-Saad
Copy link
Contributor Author

Mai-Saad commented Sep 18, 2024

3.17 Results on PR now https://filetransfer.io/data-package/tncWYOQg#link failed test is due to DB error (which shall be ignored) .. @wp-media/engineering-plugin-team can anyone please pull the branch and npm run test:smoke there to confirm results.
Screenshot from 2024-09-18 11-58-35

Note: sometimes and probably with low internet connection, tests may fail and timeout

@Mai-Saad Mai-Saad self-assigned this Sep 18, 2024
@Mai-Saad Mai-Saad requested a review from Khadreal September 18, 2024 13:59
@hanna-meda
Copy link
Contributor

hanna-meda commented Sep 18, 2024

@Mai-Saad, same results as with you on my end, 7 passed 1 failed, but a different one. Test results below:
hannameda-wp-rocket-e2e-test-results-cucumber-report-html-2024-09-18-18_00_21.pdf

Screenshot 2024-09-18 at 18 04 22

@MathieuLamiot
Copy link
Contributor

A couple of failing tests because of the already existing table errors. So I'd say good on my side.
debug-roll-back-from-the-tools-tab.log
debug-enable-all-features.log

A few notes:

  • This must be prioritized: Only rename debug.log with errors relating to WPR #67 ; it is the main cause of failing smoke tests currently it seems. Added the label and will prioritize
  • previous_stable must be 3.16 or above, otherwise import/export tests will fail because of the old UI of enabling/disabling separate cache.
  • If using up-to-date dependencies (with npm update), then the backstop reference creation fails. we will need a dedicated GH issue to follow-up on dependency updates, but this is not for now.

@MathieuLamiot MathieuLamiot requested a review from a team September 18, 2024 20:45
@Mai-Saad Mai-Saad removed the request for review from Khadreal September 19, 2024 02:07
@Mai-Saad Mai-Saad changed the title (WIP) Enhancement/refactor smoke test Enhancement/refactor smoke test Sep 19, 2024
@Miraeld
Copy link
Contributor

Miraeld commented Sep 19, 2024

It works on my side, with errors mentioned before by @MathieuLamiot .
Good on my side too.

@Miraeld Miraeld self-requested a review September 19, 2024 10:39
@Mai-Saad Mai-Saad merged commit 3bf80c5 into develop Sep 19, 2024
2 checks passed
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.

Rework code using waitForLoadState('load'
4 participants