Bug 1286686 - Initial move of Treeherder-Tests into /selenium folder by rbillings · Pull Request #2302 · mozilla/treeherder

rbillings · 2017-03-29T21:27:17Z

Moving the contents of the Treeherder-Tests repo into the /selenium folder. Tests will not run automatically, as Travis will need to be set up. This is just to get the content into the primary repo going forward so that the Treeherder-Tests repo can be decommissioned.

This change is

edmorley · 2017-03-29T22:28:34Z

Thank you for opening the PR.

If the tests aren't actually run at all, should we wait to merge the PR until they are hooked up?

It's just that there are lots of files in this PR that will be deleted straight away (eg .travis.yml, .gitignore, Jenkinsfile and so on) so I don't think it makes sense to commit the non-python files to the repo in the first place.

edmorley · 2017-03-29T22:32:27Z

Also the Travis run is failing.

rbillings · 2017-03-29T22:32:51Z

Hooking them up will require work outside of this PR, but this needs to come first. You have a point about the travis/gitignore/jenkinsfile - I thought I'd leave them in as placeholders. @davehunt what do you think?

edmorley · 2017-03-29T22:37:11Z

Sorry I should have asked: is the intention to convert them to tests that run on commit in Treeherder's Travis run, or are they still being run in Jenkins?

davehunt · 2017-03-31T10:43:16Z

I was originally thinking that we'd migrate the tests directly into the existing suite and get them running in Travis from the start. That said, moving the existing tests into this repository more-or-less untouched and continuing to run them in Jenkins is an interesting idea. It means the two suites would co-exist but in the same repository, making it easier to manage the migration than it would be between two independent repositories.

I would suggest at least putting these beside the selenium folder instead of within it. This is because the 'selenium' folder is already being used in Travis, and adding content will likely introduce build failures (as has done here). Maybe we could put these into a 'jenkins' folder, and then migrate them one-by-one into the 'selenium' folder? Both of these could have associated Jenkins jobs during the migration.

davehunt · 2017-03-31T10:44:20Z

tests/selenium/.travis.yml

@@ -0,0 +1,10 @@
+language: python


Exclude this file as it's only used from the root of a repository, and Treeherder already has one that runs the appropriate linting and tests.

davehunt · 2017-03-31T10:45:04Z

tests/selenium/LICENSE

@@ -0,0 +1,3 @@
+This Source Code Form is subject to the terms of the Mozilla Public


Exclude this file as it's covered by LICENSE.txt in the top level of the repo.

davehunt · 2017-03-31T10:46:30Z

tests/selenium/README.md

@@ -0,0 +1,69 @@
+# Tests for Mozilla's Treeherder


Let's reword the README to the context of a director rather than a repository. Also, we can remove all the badges.

rbillings · 2017-04-07T15:02:31Z

@davehunt please review when you get the chance.

edmorley · 2017-04-07T15:17:42Z

I'm not sure it makes sense to commit the jenkins specific files at all?

davehunt · 2017-04-07T15:22:33Z

I'm not sure it makes sense to commit the jenkins specific files at all?

These are still needed while we run the tests in Jenkins. Once they're migrated to the Travis CI builds, we can remove them.

davehunt · 2017-04-07T15:23:17Z

I had suggested using a sibling directory to 'selenium' so these tests are not attempted to be run by Travis CI (which is failing).

davehunt · 2017-04-07T15:23:42Z

See #2302 (comment)

edmorley · 2017-04-07T15:26:06Z

I mean that perhaps it would be better to leave the files in their original repo whilst they are still being run on jenkins, and only commit files here as and when they are converted to run in Travis.

davehunt · 2017-04-07T15:28:16Z

I mean that perhaps it would be better to leave the files in their original repo whilst they are still being run on jenkins, and only commit files here as and when they are converted to run in Travis.

That would mean two active repositories. Also it means Jenkins would be checkout out treeherder-tests, and then also having to checkout treeherder. This isn't much of an issue, but the git revision would be for the test repo rather than the application repo. FWIW we're only talking about Jenkinsfile here.

edmorley · 2017-04-07T15:41:03Z

Sorry reading back, my responses have been a bit disjointed. Hopefully this is more complete :-)

It seems like there are a few possible approaches:

Keep running on jenkins from the original repo for now, and only commit individual test files here once converted and running in Travis.
Keep running on jenkins from the original repo for now, but still commit the test-only files here (but disabled, so they won't be running in Travis). Later the tests in this repo will be fixed/enabled on Travis, and the jenkins jobs stopped.
Move everything to this repo now and point jenkins at this repo instead of the original one. None of the tests will run in Travis initially, since the files will be in an excluded directory. Later they will be moved over to Travis and the jenkins jobs stopped.

Options 2/3 seem preferable for the reason that it makes it easier for someone to fix the tests up, since they are already in this repo.

However option 3 clutters this repo with things that are going to be removed soon, and means having to change jenkins configs to point at this repo (so extra work). The only advantage I can see with option 3 is that it avoids divergence between the repositories, however since the treeherder-tests repo is (or should be IMO) feature-frozen, I don't think that makes any difference?

As such, option 2 seemed the next best?

Or am I overlooking something? :-)

davehunt · 2017-04-07T16:16:23Z

Or am I overlooking something? :-)

I think you've summed things up nicely. My concern over option 2 is that the tests will exist in both repositories concurrently. Whilst we're not planning to add any tests, if we encountered a failure we may need to fix or improve a test. This would either require two repositories to be updated, or for them as you say to become divergent. My personal order of preference would be option 1, followed closely by option 3, and then option 2. That said, I'm not actively working on this, so I'm happy for those that are to make the decision. What I would say is that we should at all costs avoid introducing tests in the Travis CI build until they are considered stable and unlikely to cause failures.

rbillings · 2017-04-11T16:23:41Z

Reading over the comments I would make this suggestion:

Move all of the treeherder-repo + Jenkins files into a new folder "jenkins" outside of /selenium so Travis won't fail.
Update the Jenkins path to run from the new folder- a minimal amount of work to update the path, so that tests can continue to run from this repo & not have two separate active repos
Move tests back into the /selenium folder in the future as they are updated to work with Travis [future PRs]

I'm happy to update this to do the above [outside of updating the Jenkins path which falls on @davehunt ] if this sounds like a decent plan.

rbillings · 2017-04-14T14:28:55Z

@edmorley - thumbs up on the suggestion above or should I schedule a meeting to chat about this?

edmorley · 2017-04-14T14:44:59Z

Sounds fine to me :-)

rbillings · 2017-04-18T18:00:48Z

Ready for review.

edmorley

Many thanks for updating this :-)

Unfortunately the Travis run is failing (normally we wait until it's green before requesting review). If you need any help fixing it just say.

Also, could you adjust the commit message? eg:
https://groups.google.com/forum/#!topic/mozilla.dev.platform/qERnoJniCds

edmorley · 2017-04-18T22:28:21Z

tests/jenkins/Jenkinsfile

+    SAUCELABS_API_KEY = credentials('SAUCELABS_API_KEY')
+  }
+  stages {
+    stage('Lint') {


Our Travis run already lints these files, so this can be removed :-)

edmorley · 2017-04-18T22:29:27Z

tests/jenkins/README.md

@@ -0,0 +1,55 @@
+# Tests for Mozilla's Treeherder


I think we can just delete this file - most of it is no longer relevant - and the advice for contributing doesn't really make sense since the Jenkins files are now feature-frozen.

edmorley · 2017-04-18T22:29:49Z

tests/jenkins/requirements/flake8.txt

@@ -0,0 +1 @@
+flake8==3.3.0


This can be removed

edmorley · 2017-04-18T22:31:51Z

tests/jenkins/.gitignore

@@ -0,0 +1,7 @@
+__pycache__


The .gitignore in the root of the repo makes most of this redundant, and I believe even the "results" entry isn't needed, since people won't be running the Jenkins tests locally (and a dirty git directory doesn't matter in a one-off CI environment that gets deleted at end of run).

Are you suggesting that I remove the entire file as it is redundant, or just certain lines?

Ah sorry, the entire file

edmorley · 2017-04-18T22:34:35Z

tests/jenkins/requirements/tests.txt

@@ -0,0 +1,6 @@
+mozlog==3.4


Let's move this from tests/jenkins/requirements/tests.txt to tests/jenkins/requirements.txt, since it will otherwise be the only file in the requirements directory.

Also, this will need a # pyup: ignore file on the first line of the file, or pyup-bot will very persistently try to update the dependencies in this file.

edmorley · 2017-04-18T22:35:31Z

tests/jenkins/tox.ini

+[testenv]
+passenv = DISPLAY PYTEST_ADDOPTS PYTEST_BASE_URL SAUCELABS_API_KEY SAUCELABS_USERNAME \
+    JENKINS_URL JOB_NAME BUILD_NUMBER
+deps = -rrequirements/tests.txt


The path here will need adjusting to -rrequirements.txt after the rename.

edmorley · 2017-04-18T22:36:00Z

tests/jenkins/tox.ini

+    --log-raw=results/{envname}.log \
+    {posargs}
+
+[testenv:flake8]


Let's remove the flake8 parts.

edmorley · 2017-04-18T22:42:10Z

Sorry forgot to say - could you squash the commits too? :-)

davehunt · 2017-04-20T14:02:55Z

I'd suggest excluding the new jenkins folder from the python-linters job and adding --ignore=tests/jenkins/ to the python-tests-main job in Travis. As we move the tests into the selenium folder we'll need to ensure they abide by the linting rules.

rbillings · 2017-04-25T19:15:50Z

I am guessing that my changes to the .travis.yml file have incorrect syntax :(

davehunt · 2017-04-26T09:14:49Z

.travis.yml

        - source ~/venv/bin/activate
        - pip install --disable-pip-version-check --upgrade pip==8.1.1
+      exclude:
+        tests/jenkins/


To exclude this you'll need to add the path to the setup.cfg file for pycodestyle, flake8, and isort.

davehunt · 2017-04-26T09:16:30Z

.travis.yml

        - ./bin/vendor-libmysqlclient.sh ~/venv
        # Create the test database for `manage.py check --deploy`.
        - mysql -u root -e 'create database test_treeherder;'
+      --ignore=tests/jenkins/


This needs to be moved to the py.test command line in the script section. You'll see there's already arguments such as --ignore=tests/selenium/ so you just need to add another one for this path.

davehunt · 2017-04-28T09:25:32Z

I am guessing that my changes to the .travis.yml file have incorrect syntax :(

It's only isort that's failing now. Maybe there's something wrong with the skip syntax. I'll see if I can replicate it locally. Worst case is we fix up the few issues, which would be a necessary improvement for moving the code into the selenium directory anyway.

davehunt · 2017-04-28T09:34:38Z

It looks like we just need to remove the trailing forward slash on the skip value.

davehunt · 2017-04-28T09:35:55Z

setup.cfg


 [isort]
-skip = .git,__pycache__,.vagrant,node_modules,migrations
+skip = .git,__pycache__,.vagrant,node_modules,migrations,tests/jenkins/


It appears that the trailing / causes isort to not skip this directory. Probably to be consistent let's remove this from all appearances of this directory in the file.

Yeah just adding ,jenkins should do the trick :-)

edmorley

Thank you for the updates - very close now! :-)

edmorley · 2017-04-28T09:41:39Z

tests/jenkins/tests/test_filter_by_date.py

+    page = TreeherderPage(selenium, base_url).open()
+    top_datestamp = page.result_sets[0].datestamp
+    bottom_datestamp = page.result_sets[9].datestamp
+    daterange = page.open_date_range(2016-05-06, 2016-05-07).open()


Should this date be a string instead?

This test was from an old draft- so glad you caught this and the filter_by_date as the work was already in other tests. Deleted!

edmorley · 2017-04-28T09:41:52Z

tests/jenkins/tests/test_filter_by_date.py

+    end_date = page.result_sets[9].datestamp
+
+    assert not top_datestamp == start_date
+    assert not bottom_datestamp == end_date


Missing end of file newline

edmorley · 2017-04-28T09:43:06Z

tests/jenkins/tox.ini

@@ -0,0 +1,20 @@
+[tox]
+skipsdist = true
+envlist = py27, flake8


I think the flake8 entry here is now no longer needed?

edmorley · 2017-04-28T09:44:44Z

tests/jenkins/tests/test_view_single_result.py

@@ -0,0 +1,17 @@
+# This Source Code Form is subject to the terms of the Mozilla Public
+# License, v. 2.0. If a copy of the MPL was not distributed with this
+# file, You can obtain one at http://mozilla.org/MPL/2.0/.


We used to add the licence header to all files, however now that we have the licence file in the root of the repository this is no longer necessary, so we decided to remove it to avoid the boilerplate. Could you remove the header from all files?

edmorley · 2017-05-15T14:31:57Z

I wasn't sure if this was ready for re-review or not, but I took a look anyway. I think pretty much everything is addressed apart from a few files still having the licence headers? (see #2302 (comment))

With that fixed, we can get this merged :-)

Updated README with notes about what needs to change Updated README, removed travis.yml and LICENSE files Created new jenkins folders to contain the treeherder-tests Removed duplicate lint, gitignore, updated requirements and tox files, and excluded new jenkins folder from Travis to avoid failing builds Moved exclude and ignore options for tests/jenkins so Travis will not fail Updated setup.cfg to pass Travis, removed unneeded license headers Removed extra headers and duplicate test

rbillings · 2017-05-16T22:14:32Z

OK, I think it's officially ready for review!

edmorley

Looks great - many thanks!

rbillings changed the title ~~Initial move of Treeherder-Tests into /selenium folder~~ 1286686 - Initial move of Treeherder-Tests into /selenium folder Mar 29, 2017

edmorley changed the title ~~1286686 - Initial move of Treeherder-Tests into /selenium folder~~ Bug 1286686 - Initial move of Treeherder-Tests into /selenium folder Mar 29, 2017

davehunt requested changes Mar 31, 2017

View reviewed changes

edmorley suggested changes Apr 18, 2017

View reviewed changes

rbillings force-pushed the selenium_tests branch from 4632ad8 to b3a7922 Compare April 25, 2017 19:07

davehunt requested changes Apr 26, 2017

View reviewed changes

rbillings force-pushed the selenium_tests branch from 6eacc5b to 5d3cf5b Compare April 27, 2017 15:41

davehunt reviewed Apr 28, 2017

View reviewed changes

edmorley suggested changes Apr 28, 2017

View reviewed changes

rbillings force-pushed the selenium_tests branch from 4f7786e to 4e405f3 Compare April 28, 2017 16:31

rbillings force-pushed the selenium_tests branch from e32f4c2 to 86de7ca Compare May 16, 2017 21:59

edmorley approved these changes May 16, 2017

View reviewed changes

edmorley merged commit 1d1f651 into mozilla:master May 16, 2017

mozilla deleted a comment from mozilla-autolander-deprecated Jul 26, 2017

		@@ -0,0 +1,3 @@
		This Source Code Form is subject to the terms of the Mozilla Public

Conversation

rbillings commented Mar 29, 2017 • edited by edmorley Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

edmorley commented Mar 29, 2017

Uh oh!

edmorley commented Mar 29, 2017

Uh oh!

rbillings commented Mar 29, 2017

Uh oh!

edmorley commented Mar 29, 2017

Uh oh!

davehunt commented Mar 31, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rbillings commented Apr 7, 2017

Uh oh!

edmorley commented Apr 7, 2017

Uh oh!

davehunt commented Apr 7, 2017

Uh oh!

davehunt commented Apr 7, 2017

Uh oh!

davehunt commented Apr 7, 2017

Uh oh!

edmorley commented Apr 7, 2017

Uh oh!

davehunt commented Apr 7, 2017

Uh oh!

edmorley commented Apr 7, 2017

Uh oh!

davehunt commented Apr 7, 2017

Uh oh!

rbillings commented Apr 11, 2017

Uh oh!

rbillings commented Apr 14, 2017

Uh oh!

edmorley commented Apr 14, 2017

Uh oh!

rbillings commented Apr 18, 2017

Uh oh!

edmorley left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

edmorley commented Apr 18, 2017

Uh oh!

davehunt commented Apr 20, 2017

Uh oh!

rbillings commented Apr 25, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

davehunt commented Apr 28, 2017

Uh oh!

davehunt commented Apr 28, 2017

rbillings commented Mar 29, 2017 •

edited by edmorley

Loading