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

run UI tests in drone on Firefox #31325

Merged
merged 5 commits into from
Feb 15, 2019
Merged

run UI tests in drone on Firefox #31325

merged 5 commits into from
Feb 15, 2019

Conversation

individual-it
Copy link
Member

@individual-it individual-it commented May 1, 2018

Description

run UI tests in drone on Firefox without using saucelabs

by using the selenium docker containers we are able to run tests in Firefox same as we do in Chrome. This PR

  1. sets the environment for it
  2. un-skips a couple of tests that actually do run in this version of selenium and firefox
  3. fixes one input. setting the Email address didn't work correctly in Firefox if the field was pre-filled

Limitations:

  1. this might break FF tests in travis, because it un-skips some tests. Question: do we need the travis runs with FF if we run them in drone
  2. currently we are limited to selenium 3.8.1, because 3.9.0 removed the enablePassThrough option. So without changing the docker container we are limited to Firefox 58.0

Motivation and Context

get more independent of travis & saucelabs

How Has This Been Tested?

🤖

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

@codecov
Copy link

codecov bot commented May 2, 2018

Codecov Report

Merging #31325 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##             master   #31325   +/-   ##
=========================================
  Coverage     64.84%   64.84%           
  Complexity    18324    18324           
=========================================
  Files          1198     1198           
  Lines         69580    69580           
  Branches       1283     1283           
=========================================
  Hits          45120    45120           
  Misses        24086    24086           
  Partials        374      374
Flag Coverage Δ Complexity Δ
#javascript 53.1% <ø> (ø) 0 <ø> (ø) ⬇️
#phpunit 66.2% <ø> (ø) 18324 <ø> (ø) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1665fef...a5f69f6. Read the comment docs.

@individual-it individual-it force-pushed the ui-tests-firefox-drone branch 2 times, most recently from 3d866f0 to 63d342e Compare May 2, 2018 04:16
@individual-it individual-it changed the title [WIP] run UI tests in drone on Firefox run UI tests in drone on Firefox May 2, 2018
@individual-it individual-it requested review from phil-davis and DeepDiver1975 and removed request for phil-davis May 2, 2018 05:23
@individual-it individual-it self-assigned this May 2, 2018
@individual-it individual-it requested review from patrickjahns and removed request for DeepDiver1975 May 2, 2018 05:23
@individual-it individual-it added this to the development milestone May 2, 2018
Copy link
Contributor

@phil-davis phil-davis left a comment

Choose a reason for hiding this comment

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

LGTM
I suggest that we remove Firefox test matrix entries from Travis.
chrome, IE11 and Edge on nightly Travis should be plenty, and Firefox will happen with every PR.

@phil-davis
Copy link
Contributor

phil-davis commented May 2, 2018

This will make another 14 matrix jobs in drone PR builds.
So that is something to think about.
IMO it is easy to have lots of drone bees working, we just need a solar-powered drone hive so the bees are green :)
@patrickjahns your thoughts?

@phil-davis
Copy link
Contributor

drone says it passed, but no codecov status has come. what happened???

@individual-it
Copy link
Member Author

@ownclouders rebase

@ownclouders
Copy link
Contributor

Hey! I'm GitMate.io! This pull request is being rebased automatically. Please DO NOT push while rebase is in progress or your changes would be lost permanently ⚠️

@ownclouders ownclouders force-pushed the ui-tests-firefox-drone branch from 63d342e to 146cae0 Compare May 2, 2018 06:22
@ownclouders
Copy link
Contributor

Automated rebase with GitMate.io was successful! 🎉

Copy link
Contributor

@patrickjahns patrickjahns left a comment

Choose a reason for hiding this comment

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

Please do not merge - see further comments

@patrickjahns
Copy link
Contributor

patrickjahns commented May 2, 2018

This will make another 14 matrix jobs in drone PR builds.

Exactly - I am against having both browsers being tested during a PR test run. It will just add unnecessary build time to pull requests.

I see testing different browser being deferred to async runs (nightly / weekly). The mechanism for this is in the works. You might be just a little bit ahead of your time ;-)

Personally I would keep this open and revive once the mechanisms are in place. We can at the same time investigate how we can also add browserstack onto drone for the nightly/weekly scenarios

@individual-it
Copy link
Member Author

rebased, made Firefox tests run on HTTP and skipped failing tests
@PVince81 @phil-davis @davitol what to do with that PR now?

Copy link
Contributor

@phil-davis phil-davis left a comment

Choose a reason for hiding this comment

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

Firstly, just 2 questions.

@@ -515,6 +526,18 @@ services:
when:
matrix:
USE_SERVER: true
SERVER_PROTOCOL: https

server:
Copy link
Contributor

@phil-davis phil-davis Feb 14, 2019

Choose a reason for hiding this comment

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

This has the same pipeline step name as server: above.
Only 1 of them ever gets started, but does it cause some confusion for drone.yml parsing?

Copy link
Member Author

Choose a reason for hiding this comment

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

no confusion for drones, only for humans
we need to have the same name because that becomes the hostname for that service and it should be the same for HTTP and HTTPS, to have less confusion

@@ -529,6 +552,18 @@ services:
when:
matrix:
USE_FEDERATED_SERVER: true
SERVER_PROTOCOL: https

federated:
Copy link
Contributor

@phil-davis phil-davis Feb 14, 2019

Choose a reason for hiding this comment

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

Same here, repeating the federated: step

@phil-davis
Copy link
Contributor

Maybe for now we could merge all the:

  • drone pipeline additions
  • putting the SERVER_PROTOCOL variable into the existing chrome webUI and other API acceptance... matrix entries
  • skipOnFirefox tag changes
  • select a couple of webUI test suites to also run all the time on Firefox (that will ensure that we know if something obvious regresses on Firefox) - maybe webUILogin and webUIRenameFiles - that will ensure that it can login and do something basic on the files page.

@individual-it
Copy link
Member Author

or running all @smokeTest tagged tests on FF?

@phil-davis phil-davis dismissed patrickjahns’s stale review February 14, 2019 06:26

Lots of code changes and discussion, so review is needed again

@individual-it individual-it force-pushed the ui-tests-firefox-drone branch 4 times, most recently from 2ebc4bb to 3c4ce3d Compare February 14, 2019 06:47
Copy link
Contributor

@phil-davis phil-davis left a comment

Choose a reason for hiding this comment

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

LGTM
And we can have a followup issue to investigate the test scenarios tagged skipOnFirefox and try to find out why they fail (real problem on Firefox or Firefox webDriver "feature"...)

@patrickjahns
Copy link
Contributor

Just a minor note - the firefox container seems to be quite old

core/.drone.yml

Line 507 in a5f69f6

image: selenium/standalone-firefox-debug:3.8.1

And it was last built 12 months ago:

$ docker images | grep firefox
selenium/standalone-firefox-debug                        3.8.1                                      c2ebdfdb9e31        12 months ago       792MB

@patrickjahns patrickjahns self-requested a review February 14, 2019 11:14
Copy link
Contributor

@patrickjahns patrickjahns left a comment

Choose a reason for hiding this comment

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

Please use a more recent firefox version

@individual-it
Copy link
Member Author

@patrickjahns see original description a newer version is not possible because of not finished implementations in the libraries

2. currently we are limited to selenium 3.8.1, because 3.9.0 removed the `enablePassThrough` option. So without changing the docker container we are limited to Firefox 58.0

@individual-it individual-it dismissed patrickjahns’s stale review February 15, 2019 03:28

not possible currently due to limitations in libraries

@individual-it individual-it merged commit 1d00fbd into master Feb 15, 2019
@delete-merged-branch delete-merged-branch bot deleted the ui-tests-firefox-drone branch February 15, 2019 03:38
@individual-it
Copy link
Member Author

@phil-davis issue opened for skipped tests #34491

@individual-it
Copy link
Member Author

@patrickjahns issue to update to a newer version of Firefox #34493

@individual-it
Copy link
Member Author

backport in #34494

@lock lock bot locked as resolved and limited conversation to collaborators Feb 15, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants