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

Remove one of in-memory/on-disk SQLite e2e runners and replace with faster test #580

Closed
aeneasr opened this issue Jul 16, 2020 · 2 comments · Fixed by #603
Closed

Remove one of in-memory/on-disk SQLite e2e runners and replace with faster test #580

aeneasr opened this issue Jul 16, 2020 · 2 comments · Fixed by #603
Assignees
Labels
feat New feature or request.

Comments

@aeneasr
Copy link
Member

aeneasr commented Jul 16, 2020

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

Right now we check if the binary starts up using e2e tests - for example the in-memory e2e tests recently established by #574

Because the implementation of in-memory and on-disk SQLite is minimal, I think we should run just one of the two - especially because the e2e tests need around 10 minutes per task to complete.

Describe the solution you'd like

Remove either the on-disk or in-memory SQLite tests from the e2e suite and find another way to introduce a test that would have caught what #574 fixed.

@aeneasr aeneasr added feat New feature or request. debt labels Jul 16, 2020
@aeneasr aeneasr added this to the v0.5.0-alpha.1 milestone Jul 16, 2020
@aeneasr aeneasr added this to To do in Maintainer's Board via automation Jul 16, 2020
@aeneasr aeneasr changed the title Improve "e2e" start up testing Remove one of in-memory/on-disk SQLite e2e runners and replace with faster test Jul 16, 2020
@tricky42
Copy link
Contributor

tricky42 commented Jul 16, 2020

I agree that it currently adds too much time. Essentially I think we need to have test coverage of the setup & initialization phase for both flavors (in-memory & on-disk). For the functional tests afterwards either one should be sufficient (do we have a favorite here?).

In the end I think we need to ensure that we catch all the different problems we found on the way to fix #574 in our normal tests.

Right now I dont know enough about the test concept of Kratos to sketch out the best setup. In general we should to cover the following specific cases for in-memory:

  • Correct substitution of shortcut value "memory" to our in-memory config. DONE as we have a Unit Test for IsSQLiteMemoryMode( ) in driver/driver_default_test.go.
  • Successful startup / initialization using our in-memory config. MISSING as this is currently done inefficiently by adding a complete E2E flavor and therefore adding 10min to each e2e run....

Having this in place we can remove "memory" flavor from e2e tests again. The e2e flavor SQLite will ensure that everything works on SQLite assuming on-disk and in-memory behave the same.

@zepatrik @aeneasr what do you think and any pointer how to do t/ where to add the startup/init test?

@aeneasr
Copy link
Member Author

aeneasr commented Jul 17, 2020

I would prefer having on-disk for e2e as it allows debugging the SQLite database when tests fail!

Successful startup / initialization using our in-memory config. MISSING as this is currently done inefficiently by adding a complete E2E flavor and therefore adding 10min to each e2e run....

I think we could either test NewDefaultDriver against different configs or alternatively do something like this:

https://github.com/ory/hydra/blob/master/cmd/root_test.go#L73

aeneasr added a commit that referenced this issue Jul 27, 2020
Maintainer's Board automation moved this from To do to Done Jul 27, 2020
aeneasr added a commit that referenced this issue Jul 27, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feat New feature or request.
Projects
Development

Successfully merging a pull request may close this issue.

2 participants