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

add basic e2e tests for stack title #4206

Merged

Conversation

shoetten
Copy link
Contributor

@shoetten shoetten commented Nov 12, 2022

  • Target version: master

Summary

I created some basic e2e tests for stack title editing & aborting it.

Some things to consider:

  • I introduced eslint to cypress tests in 8bbf6d3d7ad2c035b3792ea26c09a95d6eda81f4 if you don't want that in there, just tell me and i will rip it out
  • i changed the submit button tooltip in 439b7064dd58d5fb04e4242772516f3596f7cca6 because the old message was not making any sense to me. but it's not strictly related and will require an update from translators. so if you want that seperated out, plz tell me.
  • usually i go with the coding style already present, but here i went with data attributes to select elements, since i agree with the cypress docs, that relying on classes / ids is not a good idea

To be honest i'm not inclined to write any more tests for now, because i think the cypress setup needs some work upfront first (and also other things are more fun ;)). My two main concerns are that
a) the dev and test database is shared and tests don't clean up after themselves and
b) tests run really slow, mostly through how the setup / spec bringup is handled (my machine already takes >5min, for a tiny amount of tests)
Or am i just missing something here?

Checklist

  • Code is properly formatted
  • Sign-off message is added to all commits
  • Tests (unit, integration, api and/or acceptance) are included
  • Documentation (manuals or wiki) has been updated or is not required

Signed-off-by: Simon Hötten <s+git@hoetten.org>
Signed-off-by: Simon Hötten <s+git@hoetten.org>
Signed-off-by: Simon Hötten <s+git@hoetten.org>
Signed-off-by: Simon Hötten <s+git@hoetten.org>
@juliushaertl juliushaertl force-pushed the feature/add-basic-integration-tests branch from 829693b to 2be2afb Compare December 30, 2022 13:43
@juliushaertl
Copy link
Member

To be honest i'm not inclined to write any more tests for now, because i think the cypress setup needs some work upfront first (and also other things are more fun ;)). My two main concerns are that

Yes, fully agreed that the current setup is far from perfect. Thanks for adding the tests nevertheless and for your input.

a) the dev and test database is shared and tests don't clean up after themselves and

Yes, that is something that might need a general approach for nextcloud cypress tests. It would be quite helpful to also be able to pre-seed data with that.

b) tests run really slow, mostly through how the setup / spec bringup is handled (my machine already takes >5min, for a tiny amount of tests)

We may want to move over to the https://github.com/nextcloud/nextcloud-cypress helpers here as well as they at least speed up test setup in terms of login and session handling.

I took the liberty to rebase so we can get this merged.

Signed-off-by: Julius Härtl <jus@bitgrid.net>
@juliushaertl juliushaertl force-pushed the feature/add-basic-integration-tests branch from 7f84dda to 5958632 Compare December 30, 2022 14:42
@juliushaertl juliushaertl merged commit 5d46127 into nextcloud:master Dec 30, 2022
@shoetten shoetten deleted the feature/add-basic-integration-tests branch December 31, 2022 16:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants