Closed Bug 946533 Opened 6 years ago Closed 6 years ago

[Rocketbar] Add some tests

Categories

(Firefox OS Graveyard :: Gaia, defect)

defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED
1.3 C1/1.4 S1(20dec)

People

(Reporter: kgrandon, Assigned: kgrandon)

References

Details

(Whiteboard: [c= p=3 s= u=])

Attachments

(2 files, 2 obsolete files)

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.
Attached file Pull requests - add some basic tests (obsolete) —
Attachment #8342766 - Attachment is obsolete: true
Comment on attachment 8346709 [details] [review]
Pull Request - Add Tests

Mike - got time for a quick review?
Attachment #8346709 - Flags: review?(mike)
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)
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)
(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 :)
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 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.
Attachment #8346709 - Flags: review?(mike)
(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 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.
Attachment #8346709 - Flags: review?(mike)
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
Attachment #8346709 - Flags: review?(mike)
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 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.
Attachment #8346709 - Flags: review?(mike)
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 :)
Attachment #8346709 - Flags: review?(mike) → review+
Thanks for the review. This has landed in the rocketbar2 branch here: https://github.com/mozilla-b2g/gaia/commit/cc35d60ebc900d53712c3f3d531a5558322d5506
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Attached file Testing follow up (obsolete) —
Attached file Testing follow-up
Attachment #8348209 - Attachment is obsolete: true
Target Milestone: --- → 1.3 C1/1.4 S1(20dec)
You need to log in before you can comment on or make changes to this bug.