Add guidance for testing conventions in each language #16734
Conversation
titusfortner
requested review from
RenderMichael,
aguspe,
asolntsev,
cgoldberg,
diemol,
harsha509,
navin772,
nvborisenko and
p0deje
titusfortner
added
C-py
There was a problem hiding this comment.
Hyperlinking to each binding readme would also be beneficial.
There was a problem hiding this comment.
Yes, this is a good idea. I was going to do that, but our READMEs aren't consistent right now with what they do and don't contain, and I was thinking it might make sense to update them to mostly point to our website docs, and right now the coding agents we're targeting can't access the internet.
|
|
||
| ## Running Tests | ||
|
|
||
| Bazel creates test targets for each browser. Always use `--pin_browsers`. |
There was a problem hiding this comment.
This is relevant for CI, but for day to day inner-loop testing, I use the Visual Studio integration comes in through NUnit. Rider supports it too. It gives the same results, modulo any build differences that comes in when bazel build and dotnet build accidentally diverge.
There was a problem hiding this comment.
(comment aimed at the entire Running Tests section)
There was a problem hiding this comment.
I set //common:pin_browsers in my .bazelrc.local to ensure that I'm always using pinned browsers.
There was a problem hiding this comment.
@RenderMichael - yes, we should document working in IDEs without Bazel as well. We have a section in the current README for how to run the ruby code with RubyMine (by pointing to the ruby version generated by bazel). I agree Bazel isn't as good for interactively debugging code as VSCode/Rider is.
Bazel should be essentially calling the dotnet command under the hood, just in a hermetic way, so the results shouldn't diverge. Bazel is invaluable if you want to run tests or do builds without needing a local .NET dev environment. If all you're doing is local .NET development, I understand it can be an extra hurdle.
There was a problem hiding this comment.
Bazel is growing on me the more I understand it, it’s like Docker with a slightly different use case.
My question is, who is the target audience for each language’s respective TESTING.md file? If eg. this file is for contributors to the .NET bindings, the dev environment would be relevant. Otherwise, who would need to run a language binding’s tests? Someone who’s made changes to the grid? I feel like the 99% scenario is people who have a local env and want to test their changes.
There was a problem hiding this comment.
The dotnet/TESTING.md is specifically for contributors to .NET bindings
Essentially what things would have made it easier for you to start contributing to this code base when you first pulled it up?
It should include: here are the conventions we use, here's how to use it with your IDE, here's how to verify it builds/tests with Bazel so there aren't problems with releases, etc.
You can probably assume contributors have a local dev environment for it, and if they don't, they'll know their way around the bazel pieces well enough anyway.
There was a problem hiding this comment.
Looks like we are writing this on stones... Meaning it will be harder to "improve" it: #15536
Final word: I cloned https://github.com/microsoft/playwright-dotnet, opened in any IDE, and successfully executed ALL tests without any instructions.
There was a problem hiding this comment.
@nvborisenko this is guidance for writing tests not running them.
|
|
||
| Test files end in `_tests.py` (e.g., `visibility_tests.py`). | ||
|
|
||
| ## Running Tests |
There was a problem hiding this comment.
We can also add section to run tests directly with pytest, and the flags we require.
I will add them.
and run specific test with bazel
|
|
||
| ## Running Tests | ||
|
|
||
| Bazel creates test targets for each browser. Always use `--pin_browsers`. |
There was a problem hiding this comment.
I set //common:pin_browsers in my .bazelrc.local to ensure that I'm always using pinned browsers.
| | `@NoDriverBeforeTest` | Test needs custom capabilities or tests driver creation itself. Driver is destroyed, must use `createNewDriver(capabilities)` in the test to create one. | | ||
| | `@NoDriverAfterTest` | Test leaves browser in a bad state. Driver is restarted after the test. Also accepts `failedOnly`. | | ||
|
|
||
| For tests needing two browsers simultaneously (e.g., multi-user scenarios), create a second instance with `localDriver = new ChromeDriver()`. This driver is automatically quit after the test. |
There was a problem hiding this comment.
Nope! This will (at best) create a Chrome instance instead of the browser being used for the tests. The only reason why the browser is closed after tests is because the entire process tree is killed.
The correct way to start a new browser instance is via something like:
RemoteWebDriver.builder().oneOf(Browser.detect()).build();There was a problem hiding this comment.
Thank you! This is exactly why we need this file. (this is much more helpful than the generic warning I made about being careful hard-coding creation of new drivers)
Also, if you name it localDriver, it is automatically checked in after hook: https://github.com/SeleniumHQ/selenium/blob/trunk/java/test/org/openqa/selenium/testing/JupiterTestBase.java#L90
10 participants
Reason for PR
Each language has its own tribal knowledge right now for how it works, from annotations to helper methods to common locations for things. This is intended to document those things to make it easier to work in each language.
💥 What does this PR do?
Removes language specific testing details from primary README and moves everything to separate
TESTING.mdfiles💡 Additional Considerations
This is in draft because they were auto-generated as a place to start. I updated some of them more than others, but they can all use a lot of work. Please assist, thanks.