Closed Bug 1411090 Opened 5 years ago Closed 5 years ago

Improve RDM test reliability using --verify mode

Categories

(DevTools :: Responsive Design Mode, enhancement, P3)

enhancement

Tracking

(firefox57 wontfix, firefox58 fixed)

RESOLVED FIXED
Firefox 58
Tracking Status
firefox57 --- wontfix
firefox58 --- fixed

People

(Reporter: jryans, Assigned: jryans)

References

(Blocks 1 open bug)

Details

(Whiteboard: [designer-tools])

Attachments

(2 files, 2 obsolete files)

The recently added --verify highlights some issues in RDM tests.  Resolving these should help with intermittent failures.
Well, getting through all the issues is taken longer than I'd like.  So, I'll post some parts for now and revisit the rest later.
Comment on attachment 8922986 [details]
Bug 1411090 - Convert devtools/client/shared/devices.js to async / await.

https://reviewboard.mozilla.org/r/194174/#review199240

::: devtools/client/shared/devices.js:48
(Diff revision 1)
>  let localDevicesLoaded = false;
>  
> -// Load local devices from storage.
> -let loadLocalDevices = Task.async(function* () {
> +/**
> + * Load local devices from storage.
> + */
> +let loadLocalDevices = async () => {

Can we simplify this by doing:
async function loadLocalDevices() {
 ...
}
exports.loadLocalDevices

::: devtools/client/shared/devices.js:96
(Diff revision 1)
>    list.splice(index, 1);
> -  yield asyncStorage.setItem(LOCAL_DEVICES, JSON.stringify(localDevices));
> +  await asyncStorage.setItem(LOCAL_DEVICES, JSON.stringify(localDevices));
>  
>    return true;
> -});
> +};
>  exports.removeDevice = removeDevice;

I personally prefer to see all exports at the end of the file.
Attachment #8922986 - Flags: review?(gl) → review+
Comment on attachment 8922987 [details]
Bug 1411090 - Clear local device cache when testing.

https://reviewboard.mozilla.org/r/194176/#review199242

::: devtools/client/shared/devices.js:101
(Diff revision 1)
>  exports.removeDevice = removeDevice;
>  
>  /**
> + * Remove all local devices.  Useful to clear everything when testing.
> + */
> +let removeLocalDevices = async () => {

Same comments as part 1.
Attachment #8922987 - Flags: review?(gl) → review+
Comment on attachment 8922988 [details]
Bug 1411090 - Stop loading frame scripts in outer browser.

https://reviewboard.mozilla.org/r/194178/#review199244
Attachment #8922988 - Flags: review?(gl) → review+
Comment on attachment 8922989 [details]
Bug 1411090 - Update browser_navigation to avoid intermittents.

https://reviewboard.mozilla.org/r/194180/#review199246
Attachment #8922989 - Flags: review?(gl) → review+
Comment on attachment 8922986 [details]
Bug 1411090 - Convert devtools/client/shared/devices.js to async / await.

https://reviewboard.mozilla.org/r/194174/#review199240

> Can we simplify this by doing:
> async function loadLocalDevices() {
>  ...
> }
> exports.loadLocalDevices

Sure!  I've updated them to match.

> I personally prefer to see all exports at the end of the file.

Okay, sounds good to me to.  I've changed to this format.
The last 2 patches need more work (they cause an intermittent to become more common), so I'll refine them in a separate bug.
Pushed by jryans@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/c662b7320a43
Convert devtools/client/shared/devices.js to async / await. r=gl
https://hg.mozilla.org/integration/autoland/rev/b93a7bc80b11
Clear local device cache when testing. r=gl
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.