search

corona-warn-app / cwa-website

Corona-Warn-App website. The CWA development ended on May 31, 2023. You still can warn other users until April 30, 2023. More information:
https://coronawarn.app/en/faq/#ramp_down
Apache License 2.0
522 stars 225 forks source link

Added specific element attribute in FAQ page #3409

Closed UzaeirAzhar closed 1 year ago

UzaeirAzhar commented 1 year ago

In this PR, I added data-e2e attribute as mentioned in README.md as a best practice for FAQ page.

MikeMcC399 commented 1 year ago

@UzaeirAzhar

You are right that there is a best-practice recommendation, however we are in the ramp-down phase of this project and the best practice recommendation is more about ensuring the long-term maintainability of Cypress tests. In practice this recommendation has not been followed in the past. The test cypress/e2e/app_to_web.cy.js is already working correctly and satisfactorily. It seems like your change only affects one of the tests, so to be consistent all of the other tests would need to be changed.

Considering that the test works correctly without this PR I would suggest not to use this PR, unless there is some advantage which I have not understood.

(For future PRs you should always branch off the master branch when you create changes to submit as a PR (see https://github.com/corona-warn-app/cwa-website/blob/master/CONTRIBUTING.md#pull-request-checklist). Also it is quite helpful to first open an issue, if you have a bug or an idea for an enhancement, to check whether the team agrees with any proposed changes.)

UzaeirAzhar commented 1 year ago

@MikeMcC399 Thanks for your comment. I understood your point. I'm closing this PR and will keep in mind points you mentioned above.

MikeMcC399 commented 1 year ago

@UzaeirAzhar

Thank you for understanding my feedback! I am working on restructuring the testing documentation and I will probably take out that particular best practices recommendation and replace it instead with a link to Cypress' Best practices documentation to make it more general.