-
Notifications
You must be signed in to change notification settings - Fork 953
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
[Docs] Adds testing requirements for PR's #5950
[Docs] Adds testing requirements for PR's #5950
Conversation
Signed-off-by: Ashwin P Chandran <ashwinpc@amazon.com>
@@ -14,13 +14,17 @@ Overview | |||
- [Misc](#misc) | |||
|
|||
# General information | |||
OpenSearch Dashboards uses [Jest](https://jestjs.io/) for unit and integration tests, [Selenium](https://www.selenium.dev/) for functional tests, and [Cypress](https://www.cypress.io/) for backwards compatibility tests. | |||
OpenSearch Dashboards uses [Jest](https://jestjs.io/) for unit and integration tests, [Cypress](https://www.cypress.io/) for backwards compatibility tests and functional tests and [Selenium](https://www.selenium.dev/) for some legacy functional tests (Tests should no longer be written in Selenium). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What's test coverage of cypress test compare to selenium test
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Right now most of the tests are covered using Selenium. The goal now that we have the Test Orchestrator is that we start moving these to cypress and deperecate their selenium counter part. The effort for this is quite large so that migration will need to happen incrementally while all new tests are written in cypress, thus aiding this transition.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
* Backwards Compatibility tests: cypress tests that verify any changes are backwards compatible with previous versions. | ||
* Backwards Compatibility tests: tests that verify any changes are backwards compatible with previous versions. | ||
|
||
> Contributors submitting pull requests (PRs) to the codebase are required to ensure that their code changes include appropriate testing coverage. This includes, but is not limited to, unit tests, integration tests, functional tests, and backwards compatibility tests where applicable. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As far as I know, only unit test will contribute to test coverage. While jest_integration tests, functional tests won't. Do we have any convention that when should we add jest_integration tests and functional tests? Or I suppose some PRs which should contain this sort of tests may only include unit tests in its changes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@AMoo-Miki is working on bringing functional test coverage to OSD. For now this is a notice that maintainers can point to when a change does not have sufficient coverage to merge
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is great to see some thinking on testing.
Could we clarify code coverage and test coverage.
Let's be sure had clear actionable plan and developer guideline so contributors could follow.
We also need a plan for those feature,component,code does't have enough coverage what will be practical plan
Description
Issues Resolved
Screenshot
Testing the changes
Check List
yarn test:jest
yarn test:jest_integration