1255051 - Remove all instances of skip_under_xvfb from firefox-ui-tests

Status: RESOLVED FIXED

(Testing :: Firefox UI Tests, defect)

Description

[Henrik Skupin [:whimboo][⌚️UTC+2]]

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.

Comment 1

[Nagma Kapoor (:nagma)]

Hi it's Nagma! I'm an Outreachy applicant interested in working on this bug.

Comment 2

[Maja Frydrychowicz :impossibus (was :maja_zf) (needinfo me)]

Go for it. Feel free to ask me or :whimboo questions on #automation.

Comment 3

[Nagma Kapoor (:nagma)]

(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 4

[Nagma Kapoor (:nagma)]

Attachment: MozReview Request: Bug 1255051 - Remove skip_under_xvfb from firefox-ui-tests; r?maja_zf

Review commit: https://reviewboard.mozilla.org/r/39299/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/39299/

Comment 5

[Nagma Kapoor (:nagma)]

Attachment: MozReview Request: Test change removed

Review commit: https://reviewboard.mozilla.org/r/39301/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/39301/

Comment 6

[Maja Frydrychowicz :impossibus (was :maja_zf) (needinfo me)]

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.

Comment 7

[Maja Frydrychowicz :impossibus (was :maja_zf) (needinfo me)]

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. :)

Comment 8

[Nagma Kapoor (:nagma)]

Comment on attachment 8729213 [details]
MozReview Request: Test change removed

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/39301/diff/1-2/

Comment 9

[Nagma Kapoor (:nagma)]

Attachment: MozReview Request: Removed skip_under_xvfb def from decorators.py

Review commit: https://reviewboard.mozilla.org/r/40161/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/40161/

Comment 10

[Henrik Skupin [:whimboo][⌚️UTC+2]]

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.

Comment 11

[Nagma Kapoor (:nagma)]

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?

Comment 12

[Henrik Skupin [:whimboo][⌚️UTC+2]]

I would defer to Maja here. I do not use Mercurial and might not be the best help for such a question.

Comment 13

[Nagma Kapoor (:nagma)]

Alright thanks Henrik, I'll get in touch with Maja.

Comment 14

[Maja Frydrychowicz :impossibus (was :maja_zf) (needinfo me)]

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 15

[Maja Frydrychowicz :impossibus (was :maja_zf) (needinfo me)]

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.

Comment 16

[Nagma Kapoor (:nagma)]

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/

Comment 17

[Henrik Skupin [:whimboo][⌚️UTC+2]]

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.

Comment 18

[Maja Frydrychowicz :impossibus (was :maja_zf) (needinfo me)]

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.

Comment 19

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/149e047e916c

Comment 20

[Nagma Kapoor (: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?

Comment 21

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/149e047e916c

Comment 22

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Thanks Nagma for working on this bug!