Skip to content
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

UI testing with C# tests and Selenium with the Lombiq UI Testing Toolbox #12834

Open
Piedone opened this issue Nov 17, 2022 · 13 comments
Open

UI testing with C# tests and Selenium with the Lombiq UI Testing Toolbox #12834

Piedone opened this issue Nov 17, 2022 · 13 comments

Comments

@Piedone
Copy link
Member

Piedone commented Nov 17, 2022

This is to gather feedback on to whether continue with the initial extended PoC I've done under #11194. So, please share your thoughts, and check out the code!

Is your feature request related to a problem? Please describe.

We need some automated tests that also test the UI of the app, so we can make sure that not just the individual units work well, but also the whole app, when interacted with from a browser (or with its web APIs). For this, we currently have OrchardCore.Tests.Functional that uses Cypress to run tests.

These tests are really useful, but there are aspects that I'd change/improve:

Describe the solution you'd like

The proposal here is to use the Lombiq UI Testing Toolbox instead of Cypress, write xUnit tests, and add more of them. To see how this would look like, I added a PoC under the #11194 PR. Please check out the code and the demo screensharing but this currently does the following:

  • Runs the setup with all built-in recipes, with all supported DBs, as well as a simple MVC sample, and checks basics. - This was already the case with the current tests, but the tests are now written in C# with xUnit.
  • Tests more functionality for each recipe (depending on the recipe): Apart from the setup and admin log, it tests the following:
  • Checks are done automatically for every page to make sure there are no errors in the browser console, no errors in the Orchard log, there are no HTML validation errors, and no accessibility issues on the frontend (admin is excluded in the configuration but can be added once we make the admin accessible).
  • All test and DB combinations run both under Ubuntu and Windows.
  • There's a test to test the basics of using Media with Azure Blob Storage.
  • There's a monkey test for the admin area that tries to break the app with random interactions. This is to surface issues that we otherwise wouldn't notice. See a demo here.
  • Since the test and the app runs in the same .NET project, you can step into the web app's code during debugging, or see exceptions right away. See this demo.

Note that the PR currently runs all the tests, with all feature tests indiscriminately, so testing alone is 6-7 minutes on GitHub Actions. This is something we can fine-tune:

  • Only running a barebone of tests, not all of them or not with all feature tests.
  • Only run under Ubuntu (which is faster than Windows) for each PR, and everything only for merge commits in main (that nobody needs to wait for).
  • Make use of setup snapshots: Currently, all tests run the Orchard setup at start. Since some tests run the setup with the same recipe, the resulting DB can be cached. This is supported by the UI Testing Toolbox, but only for SQLite and SQL Server, so due to Postgres and MySQL it's currently not utilized at all, but can be just when testing with SQLite or SQL Server.

However, if we want to get most of the benefits even for PR builds, I expect it to take more time than the 1-2 minutes of Cypress tests currently.

Potential future improvements:

  • This can also pave the way to make the admin area conform to accessibility guidelines too.
  • While the linked PR doesn't yet utilize it, we could also add visual verification testing to some of the key pages to make sure that we don't mess up their styling accidentally.
  • Apart from running tests with Chrome, we can run them with Edge and Firefox too.

Describe alternatives you've considered

Extending the Cypress tests to cover the above-mentioned areas to improve. I personally don't want to dive into that though.

@sebastienros
Copy link
Member

NB: The ASP.NET repository is getting rid of all selenium tests (too many issues after updates), and moving to Playwright.

@hishamco
Copy link
Member

hishamco commented Dec 8, 2022

FYI I already did some work using Playwright few weeks ago and testing in general, hope to share it with you in upcoming weeks. Also I mention Playwright here

One more thing to know that Playwright doesu n't support xUnit at the time I playing with it, then logged an issue in their repo, but it's closed :(

@Skrypt
Copy link
Contributor

Skrypt commented Dec 14, 2022

I think Playwright is the next thing we should look at. Though we need a POC for that and we need to see what is missing in Playwright because it is probably less mature than everything else out there.

@Piedone
Copy link
Member Author

Piedone commented Dec 15, 2022

@sebastienros can you point to something that shows details about that move to Playwright?

Getting to the same point where we are now with these tests, but with Playwright, would be a huge amount of work, so I suggest not starting with that :D.

@sebastienros
Copy link
Member

@Piedone
Copy link
Member Author

Piedone commented Dec 15, 2022

Thanks!

@Piedone
Copy link
Member Author

Piedone commented Jan 13, 2023

Last call: Please let me know if you think this is worth continuing working on. If not, I'll close the issue and PR.

@sebastienros
Copy link
Member

I don't want to decide. I am personally afraid of the complexity of the solution, the new dependencies (selenium, which we removed) and don't see much value in changing right now.

But I don't write functional tests, so I'll let those who write/fix them to decide. I just want to ensure this is not adding too many complexity and maintenance burden.

@sebastienros sebastienros added this to the backlog milestone Jan 19, 2023
@hishamco
Copy link
Member

I might create a PR for UI with Playwright which I play with it few months ago, but again there're some cool things that done by Lombiq that we can add to the new OrchardCore.Tests.UI

@Piedone
Copy link
Member Author

Piedone commented Jan 19, 2023

Well, we currently also have dependencies for functional tests, just as doing them in Playwright would add them, and any approach adds complexity over not having such tests. Compared to what we have now, the PoC I've created adds complexity, because it's testing a lot more things.

@hishamco
Copy link
Member

FYI dotnet/aspnetcore#45682

@Piedone
Copy link
Member Author

Piedone commented Jun 19, 2024

I think the goal should be that every module feature should have at least one UI test as well. It doesn't mean that every possible functionality should be UI tested, but every module feature should have a corresponding smoke test at least, that makes sure that the feature is not broken in some very obvious, fundamental way (that starts with making sure that enabling the feature still works, what can break too, and if it has recipes, executing them also works).

@MikeAlhayek MikeAlhayek modified the milestones: 2.1, 2.x Nov 12, 2024
@MikeAlhayek MikeAlhayek added the P2 label Nov 12, 2024
Copy link
Contributor

We triaged this issue and set the milestone according to the priority we think is appropriate (see the docs on how we triage and prioritize issues).

This indicates when the core team may start working on it. However, if you'd like to contribute, we'd warmly welcome you to do that anytime. See our guide on contributions here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants