Closed Bug 946533 Opened 6 years ago Closed 6 years ago
[Rocketbar] Add some tests
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 on attachment 8346709 [details] [review] Pull Request - Add Tests Mike - got time for a quick review?
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.
(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.
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  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: . 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.  https://bugzilla.mozilla.org/show_bug.cgi?id=907177  https://bugzilla.mozilla.org/show_bug.cgi?id=924170#c6
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.
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
Follow-up landed in feature branch: https://github.com/mozilla-b2g/gaia/commit/01aa9cb74859ae9d45fc3ae1bfff127800de8ff6
Comment on attachment 8350012 [details] Testing follow-up >https://github.com/KevinGrandon/gaia/commit/32c6c50a4c320b31c1100ee7f8fe64c8acfa4eff
Attachment #8350012 - Flags: review+
You need to log in before you can comment on or make changes to this bug.