search

ory / examples

A curated collection of examples and solutions created and maintained by the Ory Community.
https://www.ory.sh/community/
Apache License 2.0
135 stars 67 forks source link

feat: add tests to django example #42

Closed vinckr closed 1 year ago

vinckr commented 1 year ago

Add automated tests to the django example. The same template should then be usable for the other examples.

fyi @kevgo should we also add text-runner in this PR? it would check that all commands are correct right?

Related Issue or Design Document

Checklist

Further comments

vinckr commented 1 year ago

Test is passing 🎉 Can you take a look maybe @kevgo ?

kevgo commented 1 year ago

And yes, I will focus on adding Text-Runner here and elsewhere very soon. I was tinkering all weekend on Text-Runner to update it!

vinckr commented 1 year ago

Thanks for your review Kevin, really appreciate the detail! I did not have time to look into it, but will do so now (and probably tomorrow :grin:)

Good point about moving the test into the directory. I went by how it is in the ory/docs repo (https://github.com/ory/docs/tree/master/tests) but in this repo it probably makes more sense to have it in the app dir (not sure why it is this way in /docs, maybe to make the CI easier).

Another thing I am not 100% sure of is if we should run all tests for all examples in one workflow or if we should make a dedicated workflow for each example. If we have only one workflow, its quite a long test (like in ory/docs). If we have one workflow for each example we have a lot of workflows over time. Intuitively I am leaning towards the second option, wdyt?

kevgo commented 1 year ago

The repo ory/docs contains a single codebase. It has a src folder containing the source code and a tests folder containing the tests. That makes sense.

The repo here (ory/examples) is a monorepo. It contains many different codebases (the various example apps). Each code base (example app) is pretty independent of each other. They are in different programming languages (Go, Python, TypeScript) and are run separately from each other. Each codebase (example app) in this repo should have its own src and tests folder.

So, long story short, please move the tests for the Django example into a django-ory-cloud/tests folder (after you apply my comments and suggestions).

Good question around how we set up testing. I think your second option makes more sense, especially given the fact that each codebase uses a different technology and therefore might need a different docker image to run.

We can also do your first option (test all codebases in one workflow), with the performance optimization that we only run the test of codebases that are being changed in that PR. I.e. don't run tests for codebases that don't get changed in a PR. This should keep that workflow short.

vinckr commented 1 year ago

Thanks - lots of other topics this week but will try to finish it by EOW. If you have bandwidth, I also appreciate the help :-)

vinckr commented 1 year ago

Test is failing since the line discussed here is missing: https://github.com/ory/examples/pull/42#discussion_r943641956

https://github.com/ory/examples/runs/7803148964?check_suite_focus=true#step:8:35

vinckr commented 1 year ago

Thanks a lot for the help and the code review @kevgo 🙌. Tests are now passing again, and run from the correct folder. This will make a great exemplary test case that we can use for the rest of examples in here.

For the next couple of tests: Does it make sense in your opinion to add a step to playwright.yml for each example like so:

      - name: Run Playwright tests for django-ory-cloud
        run: cd django-ory-cloud && make test

I think we should run all the tests for all examples when we push - since there might be something that has changed in Ory that breaks the examples unrelated to new changes we are making. Does that make sense?

kevgo commented 1 year ago

Sorry that I didn't catch the failing test, it "was working on my machine" haha 😆 I think I don't see CI runs (and failures) because I'm still an outside contributor to this repo.

I think we should run all the tests for all examples when we push - since there might be something that has changed in Ory that breaks the examples

That's a good point. Ideally we would run the examples as part of the test suite for Ory! I'll have to think how to make that happen using GH Actions.

Until we have that set up, you are right that we should run all examples at all times to catch breaking examples as quickly as possible. If that's the goal, I would recommend running each end-to-end test in a separate VM by making a dedicated YML file for each end-to-end test:

This has the advantages that each test gets a fresh machine (without leftovers from the previously running tests), all tests run in parallel, we have dedicated build statuses for each test in PRs and dedicated status badges for each test in README.md. This setup let's us know with one glance which examples work and which ones are broken. WDYT?

vinckr commented 1 year ago

Makes sense! I wanna start with the next test next week. I renamed this one to E2E Tests for django-ory-cloud and then we have a .yml for each. If we can run everything as part of the official test suite that would be good, but we should make sure to prevent that a broken community example doesn't block updates in /docs.

I also added you as maintainer here, but we could also add you to the ory mod team, then you have access to moderate all discussions as well.

vinckr commented 1 year ago

Everything but two points is resolved 🙏

vinckr commented 1 year ago

Alright added the last changes, thanks for the patience and reviews 🙏 Maybe we can make this a template that you just get with a new example? make example and it creates a bunch of template files for README/playwright (and possibly more?)

kevgo commented 1 year ago

Maybe we can make this a template that you just get with a new example?

I propose we do a few examples and then see how much overlap there is between them. If it's just a few files (readme, playwright config), it might be easier to just copy-and-paste an existing example when one creates a new one.