Skip to content
This repository has been archived by the owner on Nov 3, 2021. It is now read-only.

bug 929643 - Python set device tests to fail on device type, remove fina... #13068

Closed
wants to merge 1 commit into from

Conversation

zacc
Copy link

@zacc zacc commented Oct 24, 2013

...l traces of xfail

@bebef1987
Copy link
Contributor

Why no Travis CI?

@zacc
Copy link
Author

zacc commented Oct 24, 2013

It did run although I've re-started it
https://travis-ci.org/mozilla-b2g/gaia/builds/12986775

@zacc
Copy link
Author

zacc commented Oct 24, 2013

@@ -3,4 +3,4 @@
[test_calendar_today_date.py]
[test_calendar_new_event_appears_on_all_calendar_views.py]
# Bug #877611 - Make "Select time" popup, in Calendar app, more test friendly
xfail = true
disabled = true
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why are we disabling (skipping) these tests? If they're expected to fail we should use expected = fail

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well I thought that is not a test that's going to turn green when that bug is fixed. The core problem is html structure and so forth in Gaia so even if they fixed that bug we'd still need to update the test.
Thus I thought I'd save the time of running what we know will fail!

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't look at the test, so your comment may make sense. I'm fine with disabling (skipping) tests we don't want to run at all. :)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we make the disabled reason the comment, so it's included in the reports? We can then also remove the comment.

disabled = Bug 877611 - Make "Select time" popup, in Calendar app, more test friendly

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will do it but this is pretty inconsistent, I mean you can do a reason like this but you can't do a reason for fail-if = , it's a bit crap.

@davehunt
Copy link
Contributor

Does this remove all xfail labels? If so, we should remove mention of them from the README. After merging we'll also need to update Jenkins and any documentation.

@zacc
Copy link
Author

zacc commented Oct 24, 2013

Yes it removes the defaults, too. and Jenkins needs to be fixed up. Travis is taken care of in this pull.

@zacc
Copy link
Author

zacc commented Oct 24, 2013

Force pushed the changes @davehunt

@davehunt
Copy link
Contributor

Yes it removes the defaults, too. and Jenkins needs to be fixed up. Travis is taken care of in this pull.

In that case please also update the README as mentioned.

I also just noticed that test_browser_bookmark.py has an unconditional conditional expected fail, can we fix that to be expected = fail in this pull too?

@zacc
Copy link
Author

zacc commented Oct 24, 2013

yeah that came in another commit.

@zacc
Copy link
Author

zacc commented Oct 24, 2013

Done and cancelled the Travis job for the last commit

@stephendonner
Copy link

Just a drive-by, pointy-haired comment: this is some kind of awesome :-) 👍

@@ -14,4 +13,4 @@ xfail = false
[test_clock_set_alarm_snooze.py]
[test_clock_set_alarm_time.py]
# Bug 877611 - Make "Select time" popup, in Calendar app, more test friendly
xfail = true
disabled = true
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As above regarding comment and skip reason.

@davehunt
Copy link
Contributor

Added a few more comments, sorry for missing these first time. I'd like to see the skip reasons updated, but not too concerned about the device != "desktop" vs expected = fail comments as these won't make any real difference.

@zacc
Copy link
Author

zacc commented Oct 25, 2013

all address @davehunt

@zacc
Copy link
Author

zacc commented Oct 25, 2013

@askeing

@davehunt
Copy link
Contributor

Merged in afbf45f

@davehunt davehunt closed this Oct 25, 2013
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
4 participants