Closed Bug 1142212 Opened 5 years ago Closed 4 years ago

skip_if_b2g missing from marionette package

Categories

(Testing :: Marionette, defect)

defect
Not set

Tracking

(firefox48 fixed)

RESOLVED FIXED
mozilla48
Tracking Status
firefox48 --- fixed

People

(Reporter: ato, Assigned: deckycoss, Mentored)

Details

(Keywords: pi-marionette-client, Whiteboard: [good first bug][lang=py])

Attachments

(1 file, 5 obsolete files)

The skip_if_b2g symbol is missing from the new marionette Python package.  This makes importing it slightly ugly:

    from marionette.marionette_test import skip_if_b2g

Instead it would be better if we could do:

    from marionette import skip_if_b2g

Like you can do with MarionetteTestCase:

    from marionette import MarionetteTestCase
Mentor: ato
Whiteboard: [good first bug][lang=py]
I would like to work on it as my first bug on mozilla.
(In reply to ck from comment #1)
> I would like to work on it as my first bug on mozilla.

Excellent, assigning it to you!  I'm ato on #ateam on irc.mozilla.org if you need any guidance.
Assignee: nobody → k.03chandra
Status: NEW → ASSIGNED
Hello, I would like to fix this problem. Can you assign me on this?
thanks!
Attached patch 1142212.patch (obsolete) — Splinter Review
Attachment #8655269 - Flags: review?(ato)
The attached patch file seems to be empty?
Attachment #8655269 - Flags: review?(ato)
Attached patch 1142212.patch (obsolete) — Splinter Review
fixed nothing inside.
Attachment #8655394 - Flags: review?(ato)
Attachment #8655394 - Flags: review?(ato) → review+
Assignee: k.03chandra → grapherd
Keywords: checkin-needed
Sorry Louie, but this patch doesn't apply cleanly. Please rebase it on top of mozilla-central and re-request checkin. Sorry for the inconvenience!
Keywords: checkin-needed
Attached patch 1142212.patch (obsolete) — Splinter Review
RyanVM: I'm not sure about what mean "not rebase to top", is that I didn't hg pull and update the code?

I saw that after pull and update, marionette version change to 0.19. 

thanks.
Attachment #8655394 - Attachment is obsolete: true
Attachment #8655773 - Flags: review?(ato)
Comment on attachment 8655773 [details] [diff] [review]
1142212.patch

># HG changeset patch
># User Louie Lu <grapherd@gmail.com>
># Parent  fb720c90eb49590ba55bf52a8a4826ffff9f528b
>Bug 1142212 - Fixed marionette import skip_if_b2g problem. r=ato
>
>diff --git a/testing/marionette/client/marionette/__init__.py b/testing/marionette/client/marionette/__init__.py
>--- a/testing/marionette/client/marionette/__init__.py
>+++ b/testing/marionette/client/marionette/__init__.py
>@@ -1,28 +1,29 @@
> # This Source Code Form is subject to the terms of the Mozilla Public
> # License, v. 2.0. If a copy of the MPL was not distributed with this
> # file, You can obtain one at http://mozilla.org/MPL/2.0/.
> 
> 
> __version__ = '0.19'
> 
>-from .marionette_test import MarionetteTestCase, MarionetteJSTestCase, CommonTestCase, expectedFailure, skip, SkipTest
>+from .marionette_test import MarionetteTestCase, MarionetteJSTestCase, \
>+    CommonTestCase, expectedFailure, skip, skip_if_b2g, SkipTest
> from .runner import (
>-        B2GTestCaseMixin,
>-        B2GTestResultMixin,
>-        BaseMarionetteArguments,
>-        BaseMarionetteTestRunner,
>-        BrowserMobProxyTestCaseMixin,
>-        EnduranceArguments,
>-        EnduranceTestCaseMixin,
>-        HTMLReportingArguments,
>-        HTMLReportingTestResultMixin,
>-        HTMLReportingTestRunnerMixin,
>-        Marionette,
>-        MarionetteTest,
>-        MarionetteTestResult,
>-        MarionetteTextTestRunner,
>-        MemoryEnduranceTestCaseMixin,
>-        TestManifest,
>-        TestResult,
>-        TestResultCollection
>+    B2GTestCaseMixin,
>+    B2GTestResultMixin,
>+    BaseMarionetteArguments,
>+    BaseMarionetteTestRunner,
>+    BrowserMobProxyTestCaseMixin,
>+    EnduranceArguments,
>+    EnduranceTestCaseMixin,
>+    HTMLReportingArguments,
>+    HTMLReportingTestResultMixin,
>+    HTMLReportingTestRunnerMixin,
>+    Marionette,
>+    MarionetteTest,
>+    MarionetteTestResult,
>+    MarionetteTextTestRunner,
>+    MemoryEnduranceTestCaseMixin,
>+    TestManifest,
>+    TestResult,
>+    TestResultCollection
> )
It means there have been changes to the code you’ve changed since you wrote your path.  You need to pull the latest code from central and then rebase your local patch on top of that.

Unfortunately I don’t know how to do this without Mercurial bookmarks.
Comment on attachment 8655773 [details] [diff] [review]
1142212.patch

Still looks good to me.
Attachment #8655773 - Flags: review?(ato) → review+
Attached patch Description of the change (obsolete) — Splinter Review
Attachment #8723180 - Flags: review?(ato)
Comment on attachment 8723180 [details] [diff] [review]
Description of the change

Sorry, the patch doesn't work. Will upload a new patch soon.
Sounds good. I suggest you use MozReview to submit your patch: http://mozilla-version-control-tools.readthedocs.org/en/latest/mozreview-user.html#mozreview-user -- if you run into any issues feel free to ask for help on #ateam.
Attached patch Description of the change (obsolete) — Splinter Review
Attachment #8723180 - Attachment is obsolete: true
Attachment #8723180 - Flags: review?(ato)
i can work on this
We haven't heard from sv in two weeks, so I'm assigning it to Decky for an Outreachy application.

Decky, please be sure to use Mozreview for patch submission - http://mozilla-version-control-tools.readthedocs.org/en/latest/mozreview/install.html#configuring-your-machine-to-use-mozreview

Questions always welcome. :)
Assignee: grapherd → coss
Comment on attachment 8729638 [details]
MozReview Request: Bug 1142212 - import skip_if_b2g from marionette instead of marionette.marionette_test; r?maja_zf

https://reviewboard.mozilla.org/r/39533/#review36195

Thanks for the patch. To complete it, could you also change import statements to take advantage of your change in `__init__.py`? 

e.g. `marionette.marionette_test import skip_if_b2g` should be `from marionette import skip_if_b2g`

I would just grep in testing/marionette directory to find all occurances.
Comment on attachment 8729638 [details]
MozReview Request: Bug 1142212 - import skip_if_b2g from marionette instead of marionette.marionette_test; r?maja_zf

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/39533/diff/1-2/
Attachment #8729638 - Attachment description: MozReview Request: Bug 1142212: added skip_if_b2g symbol to marionette package; r?maja_zf → MozReview Request: Bug 1142212 - import skip_if_b2g from marionette instead of marionette.marionette_test; r?maja_zf
Attachment #8729638 - Flags: review?(mjzffr)
Comment on attachment 8729638 [details]
MozReview Request: Bug 1142212 - import skip_if_b2g from marionette instead of marionette.marionette_test; r?maja_zf

https://reviewboard.mozilla.org/r/39533/#review36615

Thanks for the patch!
Attachment #8729638 - Flags: review?(mjzffr) → review+
https://hg.mozilla.org/mozilla-central/rev/5bac73523d33
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla48
You need to log in before you can comment on or make changes to this bug.