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

Frontend testing using puppeteer #8357

Merged
merged 8 commits into from
Jul 30, 2018
Merged

Frontend testing using puppeteer #8357

merged 8 commits into from
Jul 30, 2018

Conversation

juliusknorr
Copy link
Member

@juliusknorr juliusknorr commented Feb 14, 2018

TODO:

Results are pushed to s3 right now: https://s3.eu-central-1.amazonaws.com/nextcloud-ui-regression/nextcloud/server/8357/index.html

Running tests locally

cd tests/ui-regression
# adjust settings in config.js

# run all tests
node runTests.js
# run just one test suite
node runTests.js settings

Tests

installation:

  • installation page
  • auto login after installing

login:

  • log in page
  • forgot password
  • files page after login

files:

  • file list popover
  • files sidebar (comments, sharing, versions)

settings:

  • personal settings page
  • admin settings - sharing, security, theming, encryption, additional, tips-tricks
  • apps management
  • user management

files_sharing:

  • public share link
  • public share link not found

@juliusknorr juliusknorr added the 2. developing Work in progress label Feb 14, 2018
@juliusknorr juliusknorr force-pushed the ui-regression branch 11 times, most recently from 8a7e641 to b2f5841 Compare February 14, 2018 16:36
{title: 'mobile', w: 360, h: 480},
{title: 'narrow', w: 800, h: 600},
{title: 'normal', w: 1024, h: 768},
{title: 'wide', w: 1920, h: 1080}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

qhd & uhd? :)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll have a look when I the tests are running fine in drone. 👍

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also uwqhd! It starts to be more and more common nowadays (21/9 monitors) :)

@juliusknorr juliusknorr force-pushed the ui-regression branch 3 times, most recently from 1fb213b to 9660ba5 Compare March 3, 2018 16:52
@juliusknorr
Copy link
Member Author

Some input about how we should handle failing screenshot comparison would be nice. Ideally we would have the following:

  • Drone tests do not fail if screenshots are not the same
  • Upload screenshots and diff to some storage (like s3) http://plugins.drone.io/drone-plugins/drone-s3/
  • Leave a comment in the PR about what tests are failing with a link to the screenshots

@MorrisJobke If i remember this correctly, we can use secrets in drone securely and use them within pull requests, if the drone job doesn't run any script from the git repo. So I thought we can create a drone plugin that is separate from the main test run for publishing the screenshot files and commenting on github. Is there anything else that I need to be aware of here?

@MorrisJobke
Copy link
Member

@MorrisJobke If i remember this correctly, we can use secrets in drone securely and use them within pull requests, if the drone job doesn't run any script from the git repo. So I thought we can create a drone plugin that is separate from the main test run for publishing the screenshot files and commenting on github. Is there anything else that I need to be aware of here?

Sounds good. Yes - should be fine. I would still like to have a new GitHub account for this and not use the Nextcloud-bot, because this one has push permissions to the repos. On the other hand the API token should also have quite limited scope if set up correctly.

@juliusknorr juliusknorr force-pushed the ui-regression branch 12 times, most recently from c6033c1 to 116197d Compare March 20, 2018 11:53
.drone.yml Outdated
TESTS: ui-regression
publish-s3:
image: plugins/s3
endpoint: https://ci-assets.nextcloud.com.weasel.rocks
Copy link
Member

@MorrisJobke MorrisJobke Jun 21, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just drop the .weasel.rocks, as it is under the correct domain now.

@juliusknorr juliusknorr force-pushed the ui-regression branch 4 times, most recently from 225983a to ffe590f Compare July 29, 2018 22:20
Signed-off-by: Julius Härtl <[email protected]>

Reincrement network idle to 3 s

Signed-off-by: Julius Härtl <[email protected]>
@juliusknorr juliusknorr force-pushed the ui-regression branch 3 times, most recently from 009b5d7 to e6d3b65 Compare July 29, 2018 22:43
@juliusknorr
Copy link
Member Author

As discussed with @rullzer Lets get this in and fix possible risky tests after. Failing UI regression tests will not cause blocking of pr merge as of right now.

Tests that sometimes fail/existing issues:

  • install/advanced: Those will succeed once this is merged to master. See 4bd7cb7 Sadly I cannot reproduce locally so it is pretty hard to debug

  • failure caused by app menu: This is most likely an issue with the app menu right now, that is caused by moving it to flexbox (Header issues with small screens #10452 probably)
    screenshot from 2018-07-30 14-43-13

@juliusknorr
Copy link
Member Author

Failing acceptance-app-files unrelated: https://drone.nextcloud.com/nextcloud/server/9324/234 Fails on master as well

@juliusknorr juliusknorr added 3. to review Waiting for reviews and removed 2. developing Work in progress labels Jul 30, 2018
@rullzer rullzer merged commit 75589ba into master Jul 30, 2018
@rullzer rullzer deleted the ui-regression branch July 30, 2018 14:47
@MorrisJobke MorrisJobke added this to the Nextcloud 14 milestone Aug 20, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3. to review Waiting for reviews enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants