Closed Bug 1255051 Opened 4 years ago Closed 4 years ago

Remove all instances of skip_under_xvfb from firefox-ui-tests

Categories

(Testing :: Firefox UI Tests, defect)

defect
Not set

Tracking

(firefox48 fixed)

RESOLVED FIXED
mozilla48
Tracking Status
firefox48 --- fixed

People

(Reporter: whimboo, Assigned: nagma, Mentored)

References

()

Details

(Whiteboard: [lang=py][good first bug])

Attachments

(1 file, 2 obsolete files)

We no longer run our tests on Travis so this skip decorator in the firefox-ui-harness is no longer necessary. Also we can remove any usage of it from the firefox-ui-tests.
Hi it's Nagma! I'm an Outreachy applicant interested in working on this bug.
Go for it. Feel free to ask me or :whimboo questions on #automation.
Assignee: nobody → nagmakapoor
(In reply to Maja Frydrychowicz (:maja_zf) from comment #2)
> Go for it. Feel free to ask me or :whimboo questions on #automation.

Thanks I will!
Comment on attachment 8729212 [details]
MozReview Request: Bug 1255051 - Remove skip_under_xvfb from firefox-ui-tests; r?maja_zf

https://reviewboard.mozilla.org/r/39299/#review36139

Great, just one bit missing. Could you also remove the definition of skip_under_xvfb? I believe it's in decorators.py.
Attachment #8729212 - Flags: review?(mjzffr)
Comment on attachment 8729213 [details]
MozReview Request: Test change removed

https://reviewboard.mozilla.org/r/39301/#review36143

I understand that this changeset was just a test commit. That's fine. You will remove it using hg histedit after I approve all your other changes. :)
Attachment #8729213 - Flags: review?(mjzffr)
Comment on attachment 8729213 [details]
MozReview Request: Test change removed

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/39301/diff/1-2/
Attachment #8729213 - Attachment description: MozReview Request: Bug 1255051 - removed all instances of skip_under_xvfb; r?maja_zf → MozReview Request: Test change removed
Attachment #8729213 - Flags: review?(mjzffr)
Nagma, as best this should land as a single commit and not as three different ones. I assume you missed to amend follow-up changes you did.
Hi Henrik, I'm a little unfamiliar with Mercurial so my apologies I didn't mean for it to go under 3 different commits. I tried using histedit as Maja suggested and I must have made some mistake in using it to make a revision. Is there any way for me to fix this?
I would defer to Maja here. I do not use Mercurial and might not be the best help for such a question.
Flags: needinfo?(mjzffr)
Alright thanks Henrik, I'll get in touch with Maja.
https://reviewboard.mozilla.org/r/40161/#review36781

::: testing/firefox-ui/harness/firefox_ui_harness/decorators.py
(Diff revision 1)
> -def skip_under_xvfb(target):
> -    def wrapper(self, *args, **kwargs):
> +def wrapper(self, *args, **kwargs):
> -        if os.environ.get('MOZ_XVFB'):
> +    if os.environ.get('MOZ_XVFB'):
> -            raise SkipTest("Skipping due to running under xvfb")
> +        raise SkipTest("Skipping due to running under xvfb")
> -        return target(self, *args, **kwargs)
> -    return wrapper
> +return wrapper

One more change: the definition of skip_under_xvfb includes everything that is indented under `def skip_under_xvfb(target):`, so everything until `return wrapper`.

However, it seems to be the only decorator defined in `decorators.py`, so you should go ahead and delete decorators.py entirely.
Comment on attachment 8729213 [details]
MozReview Request: Test change removed

https://reviewboard.mozilla.org/r/39301/#review36783

This commit still needs to be removed. I'll email you with some help with hg histedit.
Attachment #8729213 - Flags: review?(mjzffr)
Comment on attachment 8729212 [details]
MozReview Request: Bug 1255051 - Remove skip_under_xvfb from firefox-ui-tests; r?maja_zf

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/39299/diff/1-2/
Attachment #8729212 - Attachment description: MozReview Request: Bug 1255051 - removed all instances of skip_under_xvfb from firefox-ui-tests; r?maja_zf → MozReview Request: Bug 1255051 - Remove skip_under_xvfb from firefox-ui-tests; r?maja_zf
Attachment #8729212 - Flags: review?(mjzffr)
Attachment #8729213 - Attachment is obsolete: true
Attachment #8730796 - Attachment is obsolete: true
https://reviewboard.mozilla.org/r/39299/#review36889

Looks fine to me. The only thing which I find interesting is that the mozreview id has been changed for this patch. Usually it should stay the same for the first commit.
Flags: needinfo?(mjzffr)
Attachment #8729212 - Flags: review?(mjzffr) → review+
Comment on attachment 8729212 [details]
MozReview Request: Bug 1255051 - Remove skip_under_xvfb from firefox-ui-tests; r?maja_zf

https://reviewboard.mozilla.org/r/39299/#review36985

Yay, the tests passed on try! Looks good, thanks Nagma.
Yay! Thank you Maja for all your help! I'd be interested in contributing some more, so do you have any other bugs I could work on?
https://hg.mozilla.org/mozilla-central/rev/149e047e916c
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla48
Thanks Nagma for working on this bug!
You need to log in before you can comment on or make changes to this bug.