Add a Service Book class with a shared testProject method, for scripted pipelines #23

stephendonner · 2017-08-30T06:51:03Z

@davehunt @tarekziade @rpappalax - thoughts and feedback much-desired and appreciated!

I anticipate a fair amount of cleanup needed/desired here, but this is, IMHO, very much a working prototype.

It can be seen working (i.e. getting to the same "tox" error) on https://qa-preprod-master.fxtest.jenkins.stage.mozaws.net/job/kinto.stage.stephend/15/console as in https://qa-preprod-master.fxtest.jenkins.stage.mozaws.net/job/kinto.stage/3/console

Full output (patch): https://gist.github.com/stephendonner/9a3904b1294c12b9ecc67ea42c5ced71 vs. (existing): https://gist.github.com/stephendonner/146f85214800dd25527974ac52929f5a

Since it's a shared library, we'll also need corresponding changes to call testProject() from a project's Jenkinsfile, which I've done (also rough) in mozilla-services/kinto-integration-tests#35

…y loaded as a library

davehunt

This all looks great! Please add docs and tests where possible.

davehunt · 2017-08-30T07:59:24Z

src/org/mozilla/fxtest/ServiceBook.groovy

+import groovy.json.JsonSlurperClassic;
+
+
+HttpResponse doGetHttpRequest(String requestUrl){


I like the idea of seeing if we can use the HTTP Request plugin, though that could be a future enhancement.

Futured, for now, but will file an Issue for us to get that evaluated.

@davehunt filed #25, as you've seen; thanks for the reminder/gentle prod :-)

davehunt · 2017-08-30T08:04:57Z

src/org/mozilla/fxtest/ServiceBook.groovy

+}
+
+
+return this;


Is this needed? It looks superfluous.

If you don't return "this", you can't use the module as a top namespace object in other modules, but maybe there are better ways to do it

Hmm.. it was working fine when I had a dummy method in https://github.com/davehunt/fxtest-jenkins-pipeline/blob/servicebook/src/org/mozilla/fxtest/ServiceBook.groovy without return this;

maybe that's what "package" is for :)

davehunt · 2017-08-30T08:05:43Z

src/org/mozilla/fxtest/ServiceBook.groovy

+            }
+        }
+    stage('Ship it!') {
+        if (failures == 0) {


This should be checking the size of the failures array.

Fixed, I hope!

davehunt · 2017-08-30T08:08:18Z

src/org/mozilla/fxtest/ServiceBook.groovy

+}
+
+def testProject(String name) {
+    node {


We might not want to put the node here. We're using node within the stages, and this is only needed for the exit codes on lins 158-160, so we could simple wrap those in a node. I think that would be a more efficient use of our executors.

Thanks; fixed!

davehunt · 2017-08-30T08:09:25Z

src/org/mozilla/fxtest/ServiceBook.groovy

+                )
+            }
+            echo "checked out"
+            node {


Note that this will run on any executor. If we have a specific agent or label that we wanted to run on, we should do that. I suspect we want a specific docker image.

I don't yet know much about these actual tests, and/or where/how they best should run, besides that they should run in Docker, on the slave/execution node (at least in the qa-preprod* new setup).

Can we defer that for now, or should we hold off on merging this in until we have it clarified and codified, here?

stephendonner · 2017-08-30T17:27:39Z

Latest run is https://qa-preprod-master.fxtest.jenkins.stage.mozaws.net/job/kinto.stage.stephend/19/console.

I'll work as I have time, now, on version history and code examples, in the README. Dave and I will likely pair on the tests next week.

rpappalax · 2017-09-01T23:10:22Z

src/org/mozilla/fxtest/ServiceBook.groovy

+
+
+def validURL(url) {
+    allowedOrgs = ['Kinto', 'mozilla', 'mozilla-services', 'tarekziade', 'rpappalax']


This restriction is good to be aware of. I was thinking folks could make sure tests were somewhat viable before pushing them to a mozilla* org (i.e. that tests could be triggered from any org), but this is much safer route. I suppose we can still follow that workflow in PreProdJenkins and do soak testing there (without hooking into pipeline).
If you don't need the org for tarekziade or me, I'd go ahead and remove them.

Thanks, Richard. Actually, yes: now that we're getting really close to using this in a production/production-like environment and workflow, it's probably a good time to reel the validation back a bit; I'll make that change now.

davehunt · 2017-09-05T17:39:28Z

tests/serviceBookTests.groovy

+    //     script.doGetHttpRequest = { String url -> [resp] }
+    //     def tests = script.getProjectTests('foo')
+    //     assert tests != null
+    // }


Hey @tarekziade, we're trying to mock out the getProjectTests but hit various issues with our attempts. Do you have any suggestions for how to test this?

I'll give it a shot

davehunt · 2017-09-05T18:36:20Z

README.md

+tests, and ```exit 1``` in the event of failed builds.
+
+## Examples
+```@Library('fxtest@1.7') _


This should be ```groovy and I don't think we need to include the tag in the example.

davehunt · 2017-09-05T18:39:35Z

README.md

@@ -133,6 +133,19 @@ submitToTreeherder(
 )
 ```

+## testProject


The other entries are pipeline steps, whereas this is a ServiceBook class. It feels like there should be some separation in the documentation. Perhaps a heading for pipeline steps, and a heading for ServiceBook with the testProject method documentated? We're probably close to wanting to break the documentation out to a readthedocs or similar too, as it's getting quite lenghty.

Took a stab at this, but currently the "Shared Libraries..." heading and "Pipeline Steps" and "ServiceBook Class" are all the same heading weight of "#" - let me know if that's a deal-breaker, and I'll rework the subheadings' weights to match!

davehunt · 2017-09-05T18:40:27Z

README.md

+
+def sb = new org.mozilla.fxtest.ServiceBook()  
+sb.testProject('kinto')
+


You need to close the code sample with ```, otherwise the next heading is considered code.

Fixed; thanks (and, oops!)

davehunt · 2017-09-05T18:41:49Z

README.md

@@ -158,6 +171,8 @@ writeCapabilities(

 ## Version History

+### 1.7 (2017-09-04)
+* Introduced `testProject` method, which queries Service Book for project repos.


I would reword as something like: Introduces ServiceBook class, with testProject method to execute tests for all pipelines associated with the specified project name.

Thanks - fixed.

davehunt · 2017-09-05T19:03:44Z

README.md

+
+## Examples
+```groovy
+def sb = new org.mozilla.fxtest.ServiceBook()  


Sorry, I didn't mean to remove the library import, just the version tag as that will become dated.

davehunt

r+ with comments addressed. For the headings, I think I'd prefer Pipeline Steps and ServiceBook to be under the top heading, but that would require pushing everything else down a level, which might not look great. This is why I think it's worth moving to readthedocs, as these can then be separate pages then. Would you mind filing a separate issue for that?

davehunt · 2017-09-05T19:04:03Z

README.md

@@ -155,9 +156,23 @@ writeCapabilities(
  path: 'fx51win10.json'
 )
 ```
+# ServiceBook Class


I'd drop 'Class'

stephendonner · 2017-09-05T21:24:46Z

Filed #29 to track moving the README to Read the Docs.

stephendonner added 8 commits August 29, 2017 17:38

First stab at new shared library

955832b

Moving ServiceBook.groovy to its rightful place, so it can be actuall…

168e9be

…y loaded as a library

Pull in tests using getProjectTests

68d43ab

More cleanup

bca6ce3

LMAO; amateur hour

666023e

Actually call runStage()

c40137f

Try referencing WORKSPACE specifically

2f55e8b

Restore try/catch; more cleanup

82623e0

This was referenced Aug 30, 2017

Use new shared library testProject method to run tests from Service Book mozilla-services/kinto-integration-tests#35

Merged

Write tests for and document Service Book class #24

Closed

davehunt reviewed Aug 30, 2017

View reviewed changes

stephendonner added 3 commits August 30, 2017 09:53

Address review feedback about node{} placement

7ef4e19

Braces gonna kill me

d1467e7

Check the failures array correctly

4a6e026

Add testProject example and version history for v1.7 entry to README

d2cc953

stephendonner mentioned this pull request Aug 31, 2017

Evaluate using the HTTP Request Plugin to reduce and simplify HTTP GET/POST calls to Service Book API #25

Closed

tarekziade mentioned this pull request Aug 31, 2017

Servicebook: case insensitive lookups #27

Closed

Fix Issue mozilla#27

bd3891c

stephendonner force-pushed the servicebook branch from 145f174 to bd3891c Compare September 1, 2017 19:15

rpappalax reviewed Sep 1, 2017

View reviewed changes

stephendonner and others added 3 commits September 1, 2017 16:17

Merge branch 'master' into servicebook

20fae83

Tighten up allowed Git orgs

51b3786

Add tests for validating ServiceBook GitHub URLs

5ffde43

davehunt reviewed Sep 5, 2017

View reviewed changes

Make description of testProject method more accurate/descriptive

b813c1c

davehunt suggested changes Sep 5, 2017

View reviewed changes

stephendonner added 3 commits September 5, 2017 11:46

Add closing ticks

ae8cf3c

More succinct entry for ServiceBook class and its testProject method

4b9fa33

Address review feedback for groovy example

230adaa

Rejigger docs into Pipeline Steps + ServiceBook Class

1c09a36

davehunt reviewed Sep 5, 2017

View reviewed changes

davehunt approved these changes Sep 5, 2017

View reviewed changes

stephendonner added 2 commits September 5, 2017 12:16

Drop extraneous Class

0c60dc3

Add Library include back in

2191dbe

stephendonner mentioned this pull request Sep 5, 2017

Set up Read the Docs for and move README content #29

Closed

stephendonner merged commit 58c7efc into mozilla:master Sep 5, 2017

stephendonner changed the title ~~[WIP] Add a Service Book class with a shared testProject method, for scripted pipelines~~ Add a Service Book class with a shared testProject method, for scripted pipelines Sep 5, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add a Service Book class with a shared testProject method, for scripted pipelines #23

Add a Service Book class with a shared testProject method, for scripted pipelines #23

stephendonner commented Aug 30, 2017

davehunt left a comment

davehunt Aug 30, 2017

stephendonner Aug 30, 2017

stephendonner Aug 31, 2017

davehunt Aug 30, 2017

tarekziade Aug 30, 2017

davehunt Aug 30, 2017

tarekziade Aug 30, 2017

davehunt Aug 30, 2017

stephendonner Aug 31, 2017

davehunt Aug 30, 2017

stephendonner Aug 31, 2017

davehunt Aug 30, 2017

stephendonner Aug 30, 2017

stephendonner commented Aug 30, 2017

rpappalax Sep 1, 2017

stephendonner Sep 1, 2017

davehunt Sep 5, 2017

tarekziade Sep 8, 2017

davehunt Sep 5, 2017

stephendonner Sep 5, 2017

davehunt Sep 5, 2017

stephendonner Sep 5, 2017

davehunt Sep 5, 2017

stephendonner Sep 5, 2017

davehunt Sep 5, 2017

stephendonner Sep 5, 2017

davehunt Sep 5, 2017

davehunt left a comment

davehunt Sep 5, 2017

stephendonner Sep 5, 2017

stephendonner commented Sep 5, 2017

		import groovy.json.JsonSlurperClassic;


		HttpResponse doGetHttpRequest(String requestUrl){



		def validURL(url) {
		allowedOrgs = ['Kinto', 'mozilla', 'mozilla-services', 'tarekziade', 'rpappalax']


		def sb = new org.mozilla.fxtest.ServiceBook()
		sb.testProject('kinto')

Add a Service Book class with a shared testProject method, for scripted pipelines #23

Add a Service Book class with a shared testProject method, for scripted pipelines #23

Conversation

stephendonner commented Aug 30, 2017

davehunt left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

stephendonner commented Aug 30, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

davehunt left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

stephendonner commented Sep 5, 2017