Remove bluetooth host checking in test_toggle_bluetooth_settings

RESOLVED FIXED

Status

defect
RESOLVED FIXED
5 years ago
4 years ago

People

(Reporter: martijn.martijn, Assigned: martijn.martijn)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

()

Attachments

(1 attachment)

46 bytes, text/x-github-pull-request
davehunt
: review+
Bebe
: review+
davehunt
: feedback+
jlorenzo
: feedback+
martijn.martijn
: feedback?
Details | Review
Assignee

Description

5 years ago
Per the discussion in gaia-ui-automation, called "Bluetooth tests with remote partner", it was discussed that it would be a good idea to have the bluetooth checking from the host removed, because it would be difficult to get that working from an external lab.

If we make that part of the test depend on whether a testvars variable exists, we could keep that part of the test and control it if we want to have it enabled or not.
Assignee

Comment 1

5 years ago
Posted file bluetooth
Something like this?
Attachment #8544567 - Flags: feedback?(jlorenzo)
Attachment #8544567 - Flags: feedback?(dave.hunt)
Comment on attachment 8544567 [details] [review]
bluetooth

Looks like the way to go.

If I understood correctly, we won't have a Bluetooth Host at all once we migrate to the new lab. I think we can get rid of it entirely. If we do so, we can also remove everything after:
> self.assertTrue(self.data_layer.get_setting('bluetooth.visible'))
Attachment #8544567 - Flags: feedback?(jlorenzo) → feedback+
Comment on attachment 8544567 [details] [review]
bluetooth

This would work, though I added some comments about additional assertions we could add. Also, I wonder if it's really useful to keep this. Simply not adding (or mistyping) a key to a test variables file would cause this not to run, and it would be hard to know that we're losing coverage.
Attachment #8544567 - Flags: feedback?(dave.hunt) → feedback+
Assignee

Comment 4

5 years ago
Ok, I'll just remove the bluetooth host checking part. Making the summary reflect that.
Summary: Change test_toggle_bluetooth_settings so that bluetooth host checking is depending on a testvars setting → Remove bluetooth host checking in test_toggle_bluetooth_settings
Assignee

Comment 5

5 years ago
Comment on attachment 8544567 [details] [review]
bluetooth

Ok, this should address the feedback/review.
Attachment #8544567 - Flags: review?(jlorenzo)
Attachment #8544567 - Flags: review?(dave.hunt)
Comment on attachment 8544567 [details] [review]
bluetooth

r=me with a couple of nits addressed
Attachment #8544567 - Flags: review?(dave.hunt) → review+
Assignee

Comment 7

5 years ago
Ok, addressed the nits.
Comment on attachment 8544567 [details] [review]
bluetooth

Executed adhoc job[1], we have 2 intermittent failures. Can you look at them?

[1] http://jenkins1.qa.scl3.mozilla.com/view/UI/job/flame-kk.ui.adhoc/550
Attachment #8544567 - Flags: review?(jlorenzo)
Martijn: Could you take another look at this? I'd like to include this test in our remote lab suite.
Flags: needinfo?(martijn.martijn)
Assignee

Comment 10

4 years ago
Sorry, I forgot about this bug, because I forgot to assign it to me.
I need to look at the failures mentioned in comment 8. Note that there is some comments in bug 1116744 that might help to solve that.

Also I noticed that Bebe just commented with some improvements for my pull request.
Assignee: nobody → martijn.martijn
Flags: needinfo?(martijn.martijn)
Assignee

Comment 11

4 years ago
Unfortunately, I got a test failure with the test, which seems like a bug in current master build (bluetooth basically doesn't work). I filed bug 1129693 for that.
Depends on: 1129693
Assignee

Updated

4 years ago
No longer depends on: 1129693
Assignee

Comment 12

4 years ago
Updated the pull request with Florin's comments, I kicked off a new Jenkins try run to see if the intermittent failures are still happening: http://jenkins1.qa.scl3.mozilla.com/job/flame-kk.ui.adhoc/666/
Assignee

Comment 13

4 years ago
Comment on attachment 8544567 [details] [review]
bluetooth

I don't see any intermittent failures occuring with this Jenkins try run, so I guess that failure has disappeared somehow.
Asking review for review to see if this addressed the comments he added in the pull request.
Attachment #8544567 - Flags: review?(florin.strugariu)
Attachment #8544567 - Flags: feedback?
Comment on attachment 8544567 [details] [review]
bluetooth

test is failing and we need to remove unused dependencies
Attachment #8544567 - Flags: review?(florin.strugariu) → review-
QA Whiteboard: [fxosqa-auto-backlog+]
Assignee

Comment 15

4 years ago
Comment on attachment 8544567 [details] [review]
bluetooth

I can't reproduce the failure that Florin is seeing, nor is Jenkins affected by it. Tomorrow, Florin is going to try and find out where that failure is coming from.
Attachment #8544567 - Flags: review- → review?(florin.strugariu)
Attachment #8544567 - Flags: review?(florin.strugariu) → review+
Assignee

Comment 17

4 years ago
(In reply to Dave Hunt (:davehunt) from comment #9)
> Martijn: Could you take another look at this? I'd like to include this test
> in our remote lab suite.

Dave, I think you can now include it in the remote lab suite.
Flags: needinfo?(dave.hunt)
Yep, already done. Thanks Martijn!
Flags: needinfo?(dave.hunt)
I just removed the remaining references to pybluez as it's no longer used:
https://github.com/mozilla-b2g/gaia/commit/d99f069f1ef2ff29fa6d7043cf90e65417367e33
You need to log in before you can comment on or make changes to this bug.