-
Notifications
You must be signed in to change notification settings - Fork 3
docs(contributing): add CONTRIBUTING.md file #1
Conversation
Co-Authored-By: François Chalifour <[email protected]>
2f94dea
to
44db89b
Compare
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.
apparently I didn't submit my reviews
CONTRIBUTING.md
Outdated
- Use helper functions when possible, for readability but also for maintainability (if one widget is updated, we only have to updates its helpers without touching the tests). [More about Helpers](#helpers). | ||
- Do not make assertions to know if an action was correctly performed in the browser. If an action fails (trying to click on an non-existing element for example) then WebdriverIO will automatically throw and fail the test, so asserting on it ourselves is redundant. | ||
- Only assert what you want to see on the page after an action (Is this element displayed? Is the result list correct? etc.) | ||
- You may need to add some additional steps compared to the original scenario, to wait for some elements to update for example (this was not done in the example above for simplicity). |
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.
maybe add a permalink to somewhere we do this?
General guidelines when writing tests: | ||
|
||
- Each step should be translated as a `it` function, while this is not mandatory it helps to make the scenario more readable and properly [separate each actions in Sauce Labs reports](https://user-images.githubusercontent.com/13209/62311104-56217d80-b48b-11e9-94dc-3c18b9ddc2af.png). | ||
- All actions on the browser are asynchronous, so be sure to always `await` them. **Never run multiple asynchronous commands in parallel as it can confuse some browsers (Internet Explorer)**. |
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.
Is using await really necessary in our case? Since we're using the bakedin test runner await is implicit:
https://webdriver.io/docs/setuptypes.html#the-wdio-testrunner
It can still be something we want to enforce anyway
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.
Yeah, I'm not sure their documentation is correct since all the commands still returns Promises and I had some issues when not using await
😐.
But it will be enforced by the @typescript-eslint/no-floating-promises
ESLint rule so we should not have any problem with this.
CONTRIBUTING.md
Outdated
- All actions on the browser are asynchronous, so be sure to always `await` them. **Never run multiple asynchronous commands in parallel as it can confuse some browsers (Internet Explorer)**. | ||
- Use helper functions when possible, for readability but also for maintainability (if one widget is updated, we only have to updates its helpers without touching the tests). [More about Helpers](#helpers). | ||
- Do not make assertions to know if an action was correctly performed in the browser. If an action fails (trying to click on an non-existing element for example) then WebdriverIO will automatically throw and fail the test, so asserting on it ourselves is redundant. | ||
- Only assert what you want to see on the page after an action (Is this element displayed? Is the result list correct? etc.) |
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.
Great advice.
However for the particular case of "is the element displayed?" I think it falls in the category of things wdio automatically waits for and throws if KO:
Another way to say it is:
Instead of expect(browser.$$('div').length).to.be(1)
use https://webdriver.io/docs/api/element/waitForDisplayed.html
What do you think?
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.
Yes, it has some build in helpers for this and it will automatically throw, so maybe not a good example since we are in fact in the case described above ("WebdriverIO will automatically throw and fail the test, so asserting on it ourselves is redundant"). I will update this example.
Co-Authored-By: Haroen Viaene <[email protected]>
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.
Lets do it!
…2e-tests#1) * docs(contributing): add CONTRIBUTING.md file * Apply suggestions from code review Co-Authored-By: François Chalifour <[email protected]> * docs(contributing): fix introduction phrasing * docs(contributing): add links to directories and files * Update CONTRIBUTING.md Co-Authored-By: Haroen Viaene <[email protected]> * Apply feedbacks
…2e-tests#1) * docs(contributing): add CONTRIBUTING.md file * Apply suggestions from code review Co-Authored-By: François Chalifour <[email protected]> * docs(contributing): fix introduction phrasing * docs(contributing): add links to directories and files * Update CONTRIBUTING.md Co-Authored-By: Haroen Viaene <[email protected]> * Apply feedbacks
Add contributing guidelines.
https://github.com/algolia/instantsearch-e2e-tests/blob/64ee816d74b0d50c7cc041d865ae6e8b8fce86eb/CONTRIBUTING.md