Allure setup for Coffee Cart #12
Conversation
There was a problem hiding this comment.
Review Summary
I appreciate the solid Allure integration: the Playwright reporter includes allure-playwright, tests set hierarchy and behavior labels, and most severities use recognized values. However, there is one critical blocker that can prevent the task’s verification: in tests/customerSite/cart/updateCart/cartUpdatedAfterClickingPlusButton.spec.js the test sets allure.severity('major'), which is not a recognized Allure severity. This can cause the severity to be missing from Allure’s Graphs → Severity, failing the requirement to “check that the tests’ severities are displayed.”
Critical issues to fix:
- In
tests/customerSite/cart/updateCart/cartUpdatedAfterClickingPlusButton.spec.js, replaceallure.severity('major')with one of Allure’s valid severities:blocker,critical,normal,minor, ortrivial(e.g.,allure.severity('critical')).
Minor recommendations (not blocking):
- In
playwright.config.js, make the Allure reporter options explicit to ensure a stable output path:['allure-playwright', { outputFolder: 'allure-results' }].
Once the invalid severity is corrected, the setup should meet the requirements. Nice work on the consistent metadata and reporter configuration so far!
✨ Please rate this review in the Platform's notification center1. I will be grateful for your feedback! ✨
Important
Fix the issues, commit the changes, and then re-request my review using the Re-request review button2 in the Reviewers tab.
Tip
If you have any comments about this review, please leave a message in the Add a comment section below and mention me with the @mateacademy-ai-mentor tag3.
I will read your message during my next review of your pull request. Please do not reply directly to my comments, as that feature is not supported yet.
Caution
Do not close the pull request until the review is complete. Closing it early can lead to unexpected behavior and disrupt the review process.
Footnotes
| /* Reporter to use. See https://playwright.dev/docs/test-reporters */ | ||
| reporter: [['list']], | ||
| /* Shared settings for all the projects below. See https://playwright.dev/docs/api/class-testoptions. */ | ||
| reporter: [ |
There was a problem hiding this comment.
The Playwright reporter is configured here. Consider making the Allure reporter options explicit so the output folder is guaranteed. Example: ['allure-playwright', { outputFolder: 'allure-results' }]. This helps ensure allure serve allure-results consistently finds the results and makes the intent explicit for other developers.
| reporter: [['list']], | ||
| /* Shared settings for all the projects below. See https://playwright.dev/docs/api/class-testoptions. */ | ||
| reporter: [ | ||
| ['list'], |
There was a problem hiding this comment.
This line registers the built-in list reporter — no change required. Keeping a human-readable reporter alongside Allure is useful for local runs.
| /* Shared settings for all the projects below. See https://playwright.dev/docs/api/class-testoptions. */ | ||
| reporter: [ | ||
| ['list'], | ||
| ['allure-playwright'], |
There was a problem hiding this comment.
This registers the allure-playwright reporter which satisfies the requirement to add Allure to the Playwright reporters. If you prefer explicitness, replace with ['allure-playwright', { outputFolder: 'allure-results' }] as suggested above.
| reporter: [ | ||
| ['list'], | ||
| ['allure-playwright'], | ||
| ], |
There was a problem hiding this comment.
Reporter array closes here. If you add reporter options (see previous comments), ensure the structure remains a top-level array of reporter entries, e.g. reporter: [['list'], ['allure-playwright', { outputFolder: 'allure-results' }]].
| allure.epic('Shopping Experience'); | ||
| allure.feature('Cart Update'); | ||
| allure.story('Increment coffee quantity in cart'); | ||
| allure.severity('major'); |
There was a problem hiding this comment.
The test sets allure.severity('major'), however Allure recognizes these severity levels: 'blocker', 'critical', 'normal', 'minor', 'trivial'. Using a non-standard level like 'major' may cause the test severity not to appear in Allure's Graphs → Severity tab. This would violate the description step: "Check that the tests' severities are displayed in the Graphs → Severity tab." Consider replacing 'major' with one of the recognized values (for example 'critical' or 'normal') so the severity is correctly reported.
There was a problem hiding this comment.
Review Summary
Decision: Request Changes
You’ve done a solid job adding Allure reporting and metadata across most tests, and the Playwright config correctly includes the Allure reporter. However, there’s a critical blocker that violates the task requirement to add a valid severity to each test.
Critical issue that must be fixed:
- In
tests/customerSite/cart/updateCart/cartUpdatedAfterClickingMinusButton.spec.js, the test sets an invalid severity:allure.severity('major')(see the offending line in that file). Allure only accepts:blocker,critical,normal,minor,trivial. Usingmajorcauses the severity to be missing from the Allure report (Graphs → Severity), so the requirement isn’t met. Replace it with a valid value, e.g.,allure.severity('critical').
Minor suggestion (optional, not blocking):
- Consider making the Allure results output folder explicit in
playwright.config.js, e.g.,['allure-playwright', { outputFolder: 'allure-results' }], to help CI pick up results reliably.
Once the invalid severity is corrected, the PR should meet the task requirements. Nice work on the consistent Allure labels elsewhere!
✨ Please rate this review in the Platform's notification center1. I will be grateful for your feedback! ✨
Important
Fix the issues, commit the changes, and then re-request my review using the Re-request review button2 in the Reviewers tab.
Tip
If you have any comments about this review, please leave a message in the Add a comment section below and mention me with the @mateacademy-ai-mentor tag3.
I will read your message during my next review of your pull request. Please do not reply directly to my comments, as that feature is not supported yet.
Caution
Do not close the pull request until the review is complete. Closing it early can lead to unexpected behavior and disrupt the review process.
Footnotes
| allure.epic('Shopping Experience'); | ||
| allure.feature('Cart Update'); | ||
| allure.story('Decrement coffee quantity in cart'); | ||
| allure.severity('major'); |
There was a problem hiding this comment.
This violates the task requirement: "Add a severity attribute to each test." The test sets allure.severity('major'), but major is not a valid Allure severity. Allure recognizes: blocker, critical, normal, minor, trivial. Using an unsupported value will cause the severity to be missing from the Allure report (Graphs → Severity). Please replace allure.severity('major') with a valid value (for example allure.severity('critical')). Offending line is in tests/customerSite/cart/updateCart/cartUpdatedAfterClickingMinusButton.spec.js.
There was a problem hiding this comment.
Review Summary
Educational approval to help you progress while learning. You did a great job adding Allure metadata across tests (parentSuite/suite/subSuite, epic/feature/story, and valid severities) and referencing the Allure reporter in playwright.config.js.
Critical fix needed:
playwright.config.js: The reporter entry['allure-playwright']does not explicitly write to the requiredallure-resultsfolder (Task requirement: results must be generated toallure-results). This can causeallure serve allure-resultsto fail. Suggested change:['allure-playwright', { outputFolder: 'allure-results' }][CHECKLIST ITEM #Allure reporter configuration].
Minor improvement:
- In
tests/adminSite/editMenu/coffeeCupAddedToTotal.spec.js, remove the unusedexpectimport or add assertions that use it.
Overall, your Allure annotations are solid and aligned with the requirements—nice work! With the small config tweak, your results should generate reliably for reporting.
✨ Please rate this review in the Platform's notification center1. I will be grateful for your feedback! ✨
Footnotes
| /* Shared settings for all the projects below. See https://playwright.dev/docs/api/class-testoptions. */ | ||
| reporter: [ | ||
| ['list'], | ||
| ['allure-playwright'], |
There was a problem hiding this comment.
This violates the task requirement: "Add the Allure reporter to the playwright.config.js file." and the requirement that "Allure results must be generated to allure-results when tests run." The current reporter entry ['allure-playwright'] does not explicitly set the output folder. Consider changing this reporter entry to something like:
['allure-playwright', { outputFolder: 'allure-results' }]so that test runs reliably produce results in allure-results for allure serve allure-results to work.
2 participants
No description provided.