Repository navigation
added the locator file for c# - #2855
rpallavisharma wants to merge 14 commits into
Conversation
👷 Deploy request for selenium-dev pending review.Visit the deploys page to approve it
|
PR Summary by QodoLink C# locator documentation to a shared test example
AI Description
Diagram
High-Level Assessment
Files changed (5)
|
diemol
left a comment
There was a problem hiding this comment.
Please remove the code comments, and there is also a block of code that is commented. Should it be part of the PR, or is it just left out?
|
@diemol , sorry i was waiting for information about c# methods which are not there in selenium lib as per @nvborisenko mentioned. i commented that code thats why. i will need to remove it and then update line numbers in all files. will be fixing it all in sometime. i was considering what would be best here. first i thought let code exist but in comment, but its not correct i believe. |
|
@diemol i have removed comments. for C# by.all and by.chained are not in the default lib. they are part of - https://github.com/DotNetSeleniumTools/DotNetSeleniumExtras . unsure but @nvborisenko mentioned selenium no longer support it. Let me know if anything is required to be changed here. thanks |
Code Review by Qodo
1. C# references keep forbidden indentation
|
diemol
left a comment
There was a problem hiding this comment.
@rpallavisharma can you please check the Qodo comments?
| namespace SeleniumDocs.Elements | ||
| { | ||
| [TestClass] | ||
| public class LocatorsTest : BaseTest |
There was a problem hiding this comment.
Now this tests become chrome specific. I am not sure whether we support matrix of browsers. Ig not, then it is OK.
There was a problem hiding this comment.
I actually missed that. @rpallavisharma why do we need to avoid BaseTest?
There was a problem hiding this comment.
the reason i avoid base test or inheritance or layers in any examples is, because i think examples should be standalone entity. Anyone can see them and understand what that command in selenium does and use it. People can have their own architecture.
And i checked the base test . cs file it shows chrome browser only.
Do we need to check if examples work for all browsers? why? im not sure here.
Let me know what you both think we should do with all example code being written across bindings.
i recall some historic conversation with Titus as well... not sure... now...
@diemol @nvborisenko @titusfortner
There was a problem hiding this comment.
@rpallavisharma yes, that is true but we need to do this in a separate effort.
There was a problem hiding this comment.
should i add basetest back then? let me know. i just don't like that approach :(
There was a problem hiding this comment.
Quickly reviewed, .NET examples mix the approaches. So it is OK in this particular case to not inherit from base test.
yes i would look at what qodo says. @diemol |
|
the error -SeleniumDocs.BiDi.CDP.LoggingTest.ConsoleLogs still failing after 2 retries is not related to this. |
Thanks for contributing to the Selenium site and documentation!
A PR well described will help maintainers to review and merge it quickly
Before submitting your PR, please check our contributing guidelines.
Avoid large PRs, and help reviewers by making them as simple and short as possible.
Description
added locator.cs file and added code lines in all languages
Motivation and Context
it completes locator information for csharp
Types of changes
Checklist