Add the smoke tests for reason form #2448

desurd · 2016-10-13T11:51:23Z

using protractor as test tools

mention-bot · 2016-10-13T11:51:24Z

@desurd, thanks for your PR! By analyzing the history of the files in this pull request, we identified @tardyp to be a potential reviewer.

tardyp · 2016-10-13T15:26:01Z

smokes/e2e/force.scenarios.coffee

-        browser.waitForAngular()
-        element(By.buttonText('Start Build')).click()
-        browser.get('#/builders/1')
+        reasonForce.getLink('1/')


This api name is not clear to me.

goToURL might be clearer

tardyp · 2016-10-13T15:29:04Z

smokes/e2e/force.scenarios.coffee

-        browser.get('#/builders/1')
+        reasonForce.getLink('1/')
+        reasonForce.getLink('1/force/force')
+        reasonForce.getStart()


getStart is not clear to me.

clickStartButton() would be more meaningful.

actually, what would be better is to say

forcePage.getStartButton().click()

tardyp · 2016-10-13T15:31:39Z

smokes/e2e/force.scenarios.coffee

+
+describe('', () ->
+    beforeEach(() ->
+        reasonForce =  new forceSchedulerPage()


forcePage = new forceSchedulerPage()
forcePage.go()

tardyp · 2016-10-13T15:37:30Z

smokes/e2e/force.scenarios.coffee

 describe 'force', () ->
    it 'should create a build', () ->
-        browser.get('#/builders/1')
+        reasonForce =  new forceSchedulerPage()


forcePage = new forceSchedulerPage(builderid:1, forceschedulername: 'force')

tardyp · 2016-10-13T15:39:29Z

smokes/e2e/force.scenarios.coffee

        successBuildIncrease =  ->
            lastbuild.then (lastbuild)->
                element.all(By.css('span.badge-status.results_SUCCESS'))
                .count().then (nextbuild)->
                    return +nextbuild == +lastbuild + 1
        browser.wait(successBuildIncrease, 20000)
-        browser.get('#/waterfall')
+        reasonForce.getWaterfall()


waterfallPage = new WaterfallPage() waterfallPage.go()

tardyp · 2016-10-13T15:39:51Z

smokes/e2e/reason_force.coffee

+        #return @
+
+    getLink: (link) ->
+        browser.get('#/builders/'+ link)


tardyp · 2016-10-13T15:42:47Z

smokes/e2e/reason_force.coffee

@@ -0,0 +1,88 @@
+# this file will contains the different generic functions which
+# will be called by the different tests
+#


needs to add that this is inspired by this methodology

http://www.lindstromhenrik.com/using-protractor-with-coffeescript/

tardyp

Thanks for your efforts. Its much better, but there are still some improvments

tardyp · 2016-10-17T19:16:34Z

smokes/e2e/force.scenarios.coffee

+
+            lastbuild = element.all(By.css('span.badge-status.results_SUCCESS')).count();
+            forcePage.goToURL('#/builders/1/');
+            forcePage.goToURL('#/builders/1/force/force');


goToURL is better. but actually I think it is better to hardcode the URL in the page classes.
So that would be

forcePage.go()

without the need to first go to the builders page

tardyp · 2016-10-17T19:17:14Z

smokes/e2e/force.scenarios.coffee

+
+describe('', () ->
+    beforeEach(() ->
+        forcePage =  new forceSchedulerPage();


there is the need to parametrise the page

forcePage = new forceSchedulerPage(builder:1, schedulername: 'force');

will be manage in a second time

tardyp · 2016-10-17T19:18:13Z

smokes/e2e/force.scenarios.coffee

+forcePage = null;
+waterfallPage = null;
+
+describe('', () ->


You will not need that describe, but rather put the beforeEach into the describe 'force' test

tardyp · 2016-10-17T19:18:32Z

smokes/e2e/force.scenarios.coffee

+forceSchedulerPage = require('./reason_force.coffee')
+waterFallPage = require('./waterfall.coffee')
+
+forcePage = null;


put those variables inside the describe scope

tardyp · 2016-10-17T19:19:56Z

smokes/e2e/force.scenarios.coffee

+
+
+
+    describe 'force', () ->


here you dont need that second level of describe. Only one level is needed

tardyp · 2016-10-17T19:24:08Z

smokes/e2e/force.scenarios.coffee

+                    element.all(By.css('span.badge-status.results_SUCCESS'));
+                    .count().then (nextbuild)->
+                        return +nextbuild == +lastbuild + 1;
+            browser.wait(successBuildIncrease, 20000);


This whole wait loop might abstracted inside the builderpage.

builderPage.waitNextBuildFinished(lastbuild)

tardyp · 2016-10-17T19:25:15Z

smokes/e2e/reason_force.coffee

+    goToURL: (link) ->
+        browser.get(link);
+
+    clickOnStartButton: -> 


those 3 functions are getters, so they need to be called e.g. getStartButton

tardyp · 2016-10-17T19:26:31Z

smokes/e2e/reason_force.scenarios.coffee

+            forcePage.writeAndCheckBranchName("Gerrit Branch");
+            forcePage.writeAndCheckRepo("http//name.com");
+            forcePage.writeAndCheckRevisionName("12345");
+            forcePage.clickOnCancelButton().click();


maybe we should make sure this did not create a build?
make sure it goes back to the builder page?

tardyp · 2016-10-17T19:27:08Z

smokes/e2e/waterfall.coffee

+    constructor: ->
+        browser.get('#/waterfall');
+
+    goToWaterfall: ->


go is enough. as it is supposed to be called as

waterfallPage.go()

tardyp · 2016-10-17T19:28:34Z

smokes/e2e/force.scenarios.coffee

+                        return +nextbuild == +lastbuild + 1;
+            browser.wait(successBuildIncrease, 20000);
+            waterfallPage.goToWaterfall();
+            expect(element.all(By.css('rect.success')).count()).toBeGreaterThan(0);


here we should read:

expect(waterfallPage.countSuccess()).toBeGreaterThan(0)

codecov-io · 2016-10-26T01:51:40Z

Current coverage is 86.75% (diff: 100%)

No coverage report found for master at 5f980ec.

Powered by Codecov. Last update 5f980ec...fcd8791

desurd · 2016-11-21T13:22:17Z

task is on going
some evolutions have been requested by Tardyp to have a solid & correct base before to add any new tests

tardyp · 2016-10-19T22:01:56Z

smokes/e2e/builder.coffee

+# inspired by this methodology
+# http://www.lindstromhenrik.com/using-protractor-with-coffeescript/
+
+class builderSchedulerPage


BuilderPage ?

new patchset on going

tardyp · 2016-11-29T10:25:28Z

smokes/e2e/builder.coffee

+
+    go: () ->
+        builderID = @builderID
+        browser.get('#/builders/#{builderID}/')


You can simplify by saying:

browser.get('#/builders/#{@builderID}/')

new patchset on going

tardyp · 2016-11-29T10:25:42Z

smokes/e2e/builder.coffee

+    goForce: (forcename) ->
+        builderID = @builderID
+        forceName=@forceName
+        browser.get("#/builders/#{builderID}/force/#{forceName}")


tardyp · 2016-11-29T13:50:27Z

smokes/e2e/waterfall.coffee

+
+class waterfallPage
+    constructor: ->
+        browser.get('#/waterfall');


dont go to waterfall when you instanciate the class

tardyp · 2016-11-29T13:51:46Z

smokes/master.cfg

 c['change_source'] = []
 c['change_source'].append(changes.GitPoller(
-        'https://github.com/buildbot/pyflakes.git',
+        'git://github.com/buildbot/pyflakes.git',


please keep http as this is easier to go through the proxy

tardyp · 2016-11-29T13:52:03Z

smokes/master.cfg

 # run the tests (note that this will require that 'trial' is installed)
 factory.addStep(steps.ShellCommand(command=["trial", "pyflakes"]))

+
+#slowfactory = util.BuildFactory()


please no commented code

tardyp · 2016-11-29T13:52:18Z

smokes/protractor.conf.js

@@ -2,7 +2,7 @@ exports.config = {
    allScriptsTimeout: 11000,

    specs: [
-        'e2e/*.scenarios.coffee'
+        'e2e/force.scenarios.coffee'


keep *.scenarios

tardyp · 2016-12-06T12:40:05Z

smokes/e2e/reason_force.scenarios.coffee

+        it 'should create a build', () ->
+            builder.go()
+            builder.getBuildCount().then (lastbuild) ->
+                #forcePage = builderPage.goForce()


please remove comment

tardyp · 2016-12-06T12:40:46Z

smokes/e2e/builder.coffee

+        }[builder]
+        @forceName=forcename
+
+        #@loglink = element(By.id('logLink'))


remove commented code

tardyp · 2016-12-06T12:41:01Z

smokes/e2e/force.coffee

+
+class forcePage
+    constructor: ->
+        #@loglink = element(By.id('logLink'))


tardyp · 2016-12-06T12:41:32Z

smokes/e2e/force.coffee

+        expect(reasonElement.getAttribute('value')).toBe(reason)
+
+    setYourName: (yourName) ->
+        reasonElement = element(By.css("forcefield label[for='username'] + div input"))


its not anymore the reasonElement. call it element in all the methods, and this will be fine

tardyp · 2016-12-06T12:42:19Z

smokes/e2e/force.coffee

+        reasonElement.sendKeys(projectName)
+        expect(reasonElement.getAttribute('value')).toBe(projectName)
+
+    setBranchName: (branchName) ->


I think you could factorize this code. the last four lines are the same for every methods

using protractor as test tools Signed-off-by: David Desurmont <david.desurmont@gmail.com> Signed-off-by: desurd <david.desurmont@gmail.com>

tardyp reviewed Oct 13, 2016

View reviewed changes

desurd force-pushed the mysmokes branch from fcd8791 to 658f94c Compare October 17, 2016 15:11

tardyp requested changes Oct 17, 2016

View reviewed changes

desurd force-pushed the mysmokes branch from 658f94c to 8667708 Compare October 18, 2016 15:31

sa2ajj added the please review label Oct 30, 2016

desurd force-pushed the mysmokes branch from bf74ccf to 02cbf3a Compare November 17, 2016 15:38

tardyp reviewed Nov 29, 2016

View reviewed changes

desurd force-pushed the mysmokes branch 4 times, most recently from 9293162 to 0276633 Compare December 5, 2016 15:37

tardyp requested changes Dec 6, 2016

View reviewed changes

desurd force-pushed the mysmokes branch 2 times, most recently from b67683d to ec73486 Compare December 7, 2016 15:24

Add the smoke tests for reason form

ec73486

using protractor as test tools Signed-off-by: David Desurmont <david.desurmont@gmail.com> Signed-off-by: desurd <david.desurmont@gmail.com>

tardyp approved these changes Dec 7, 2016

View reviewed changes

tardyp closed this Dec 7, 2016

tardyp reopened this Dec 7, 2016

tardyp merged commit c86127c into buildbot:master Dec 8, 2016

Add the smoke tests for reason form #2448

Add the smoke tests for reason form #2448

Conversation

desurd commented Oct 13, 2016

mention-bot commented Oct 13, 2016

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

tardyp 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

codecov-io commented Oct 26, 2016 • edited Loading

Current coverage is 86.75% (diff: 100%)

desurd commented Nov 21, 2016

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

Choose a reason for hiding this comment

codecov-io commented Oct 26, 2016 •

edited

Loading