946533 - [Rocketbar] Add some tests

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Gaia, defect)

Description

[Kevin Grandon :kgrandon]

We're trying to prototype and move fast, but it would be useful to have some unit or marionette tests to ensure we don't break things as we go. I may end up writing one quick marionette test per provider to make sure we don't break things.

Comment 1

[Kevin Grandon :kgrandon]

Attachment: Pull requests - add some basic tests

Comment 2

[Kevin Grandon :kgrandon]

Attachment: Pull Request - Add Tests

Comment 3

[Kevin Grandon :kgrandon]

Comment on attachment 8346709 [details] [review]
Pull Request - Add Tests

Mike - got time for a quick review?

Comment 4

[Mike Pennisi [:jugglinmike]]

Hey Kevin,

The tests aren't passing for me. I'll try to debug, but it may take me a bit since I need to step into a meeting shortly. Does this error log look at all familiar?

  1) contact search able to search apps from rocketbar:
     
  JavaScriptError: (17) TypeError: window.wrappedJSObject.Rocketbar is undefined
  Remote Stack:
  execute_script @undefined, line undefined
  inline javascript, line 63
  src: "      window.wrappedJSObject.Rocketbar.render();"
      at Error.MarionetteError (/home/mike/projects/mozilla/gaia/node_modules/marionette-client/lib/marionette/error.js:67:13)
      at Object.Client._handleCallback (/home/mike/projects/mozilla/gaia/node_modules/marionette-client/lib/marionette/client.js:474:19)
      at /home/mike/projects/mozilla/gaia/node_modules/marionette-client/lib/marionette/client.js:508:21
      at TcpSync.send (/home/mike/projects/mozilla/gaia/node_modules/marionette-client/lib/marionette/drivers/tcp-sync.js:94:10)
      at Object.send (/home/mike/projects/mozilla/gaia/node_modules/marionette-client/lib/marionette/client.js:455:36)
      at Object.Client._sendCommand (/home/mike/projects/mozilla/gaia/node_modules/marionette-client/lib/marionette/client.js:501:19)
      at Object._executeScript (/home/mike/projects/mozilla/gaia/node_modules/marionette-client/lib/marionette/client.js:1448:19)
      at Object.executeScript (/home/mike/projects/mozilla/gaia/node_modules/marionette-client/lib/marionette/client.js:1188:19)
      at Object.Search.openRocketbar (/home/mike/projects/mozilla/gaia/apps/search/test/marionette/lib/search.js:51:17)
      at Context.<anonymous> (/home/mike/projects/mozilla/gaia/apps/search/test/marionette/app_search_test.js:17:12)
      at Test.Runnable.run (/home/mike/projects/mozilla/gaia/node_modules/mocha/lib/runnable.js:211:32)
      at Runner.runTest (/home/mike/projects/mozilla/gaia/node_modules/mocha/lib/runner.js:372:10)
      at /home/mike/projects/mozilla/gaia/node_modules/mocha/lib/runner.js:448:12
      at next (/home/mike/projects/mozilla/gaia/node_modules/mocha/lib/runner.js:297:14)
      at /home/mike/projects/mozilla/gaia/node_modules/mocha/lib/runner.js:307:7
      at next (/home/mike/projects/mozilla/gaia/node_modules/mocha/lib/runner.js:245:23)
      at /home/mike/projects/mozilla/gaia/node_modules/mocha/lib/runner.js:269:7
      at Hook.Runnable.run (/home/mike/projects/mozilla/gaia/node_modules/mocha/lib/runnable.js:213:5)
      at next (/home/mike/projects/mozilla/gaia/node_modules/mocha/lib/runner.js:257:10)
      at /home/mike/projects/mozilla/gaia/node_modules/mocha/lib/runner.js:269:7
      at done (/home/mike/projects/mozilla/gaia/node_modules/mocha/lib/runnable.js:185:5)
      at /home/mike/projects/mozilla/gaia/node_modules/mocha/lib/runnable.js:197:9
      at Object.executeHook (/home/mike/projects/mozilla/gaia/node_modules/marionette-client/lib/marionette/client.js:370:18)
      at process._tickCallback (node.js:415:13)

Comment 5

[Mike Pennisi [:jugglinmike]]

Actually (and now my inexperience with the Search application is really showing), I can't find a reference to `Rocketbar` anywhere in the `app/` directory (outside of these tests)

Comment 6

[Kevin Grandon :kgrandon]

(In reply to Mike Pennisi [:jugglinmike] from comment #5)
> Actually (and now my inexperience with the Search application is really
> showing), I can't find a reference to `Rocketbar` anywhere in the `app/`
> directory (outside of these tests)

Hi Mike, sorry about the confusion here! The pull request is actually being opened up against a feature branch located here: https://github.com/mozilla-b2g/gaia/tree/rocketbar2

That is currently the only place you'll find the application code. If you pull upstream/rocketbar2 things should *hopefully* work :)

Comment 7

[Mike Pennisi [:jugglinmike]]

Ah, of course! Sorry, I am so used to reviewing patches for `master` that I don't pay attention to the target branch name on GitHub anymore. I'll apply the patch to that branch and continue.

Comment 8

[Kevin Grandon :kgrandon]

Comment on attachment 8346709 [details] [review]
Pull Request - Add Tests

Actually I should've probably ensured that tests passed on travis before requesting review. Seems like there's a few waitFors that I need to fix first. Sorry about the pre-mature review request.

Comment 9

[Mike Pennisi [:jugglinmike]]

(In reply to Kevin Grandon :kgrandon from comment #8)
> Comment on attachment 8346709 [details] [review]
> Pull Request - Add Tests
> 
> Actually I should've probably ensured that tests passed on travis before
> requesting review. Seems like there's a few waitFors that I need to fix
> first. Sorry about the pre-mature review request.

No worries--my response pointing out the Travis failure actually collided with your comment just now. I also should have checked that sooner. In case it helps: I am seeing the same errors reported by that failing job on Travis. Also, I'm not sure what to expect on-screen in b2g desktop when the tests run, but currently, it simply displays the Lockscreen until failure.

Comment 10

[Kevin Grandon :kgrandon]

Comment on attachment 8346709 [details] [review]
Pull Request - Add Tests

Appear to have the settings correct this time. Please take a look if you have a chance.

Comment 11

[Mike Pennisi [:jugglinmike]]

Comment on attachment 8346709 [details] [review]
Pull Request - Add Tests

Hey Kevin,

The tests are passing for me and on TravisCI, but I have some requests (I left them on the pull request itself). These concern code organization; they won't affect the functionality of the tests. I know you were interested in landing something quickly, and I hope my suggestions don't seem like nitpicks. I'm basing this feedback on large number of iterations on the Clock integration tests [1] working with :bsilverberg and :lightsofapollo. I think that the structure of the integration tests is really important--I tried to explain the reasoning to Punam while he was writing tests for the Video application: [2].

A really simple way to think of it is that the application class should define all the interaction primitives the tests need to perform so that the tests themselves do not need to use the Marionette API directly.

[1] https://bugzilla.mozilla.org/show_bug.cgi?id=907177
[2] https://bugzilla.mozilla.org/show_bug.cgi?id=924170#c6

Comment 12

[Kevin Grandon :kgrandon]

Sure, not a problem. I mostly just cranked these out and based them off of previously existing tests. I will try to address these issues though. Thanks for the review!

Comment 13

[Kevin Grandon :kgrandon]

Comment on attachment 8346709 [details] [review]
Pull Request - Add Tests

Hey Mike - Thanks for the quick review! Here is an updated patch somewhat between your ideal situation and the current implementation. Most methods have been abstracted, resulting in slim tests. Let me know what you think.

Comment 14

[Mike Pennisi [:jugglinmike]]

Comment on attachment 8346709 [details] [review]
Pull Request - Add Tests

Looks awesome, Kevin! The tests pass on my machine and on TravisCI, so this is good-to-go :)

Comment 15

[Kevin Grandon :kgrandon]

Thanks for the review. This has landed in the rocketbar2 branch here: https://github.com/mozilla-b2g/gaia/commit/cc35d60ebc900d53712c3f3d531a5558322d5506

Comment 16

[Kevin Grandon :kgrandon]

Attachment: Testing follow up

Comment 17

[Kevin Grandon :kgrandon]

Follow-up landed in feature branch: https://github.com/mozilla-b2g/gaia/commit/01aa9cb74859ae9d45fc3ae1bfff127800de8ff6

Comment 18

[Kevin Grandon :kgrandon]

Attachment: Testing follow-up

Comment 19

[Dale Harvey (:daleharvey)]

Comment on attachment 8350012 [details]
Testing follow-up

>https://github.com/KevinGrandon/gaia/commit/32c6c50a4c320b31c1100ee7f8fe64c8acfa4eff