982140 - Add automated test for removing properties from the manifest

Status: RESOLVED FIXED

(DevTools Graveyard :: WebIDE, defect)

Description

[Mihaela Velimiroviciu (:mihaelav)]

Steps:
1. Open App Manager
2. Load an app
3. Remove a property from the app's manifest

Expected result:
The property is correctly removed from the manifest

Comment 1

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

Did you click "Save" after removing the property?

Comment 2

[Mihaela Velimiroviciu (:mihaelav)]

This is a tracking bug for adding a new automated test (removing a property) for the manifest editor.

Comment 3

[Mihaela Velimiroviciu (:mihaelav)]

Attachment: remove property test

Comment 4

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

Comment on attachment 8390559 [details] [diff] [review]
remove property test

Review of attachment 8390559 [details] [diff] [review]:
-----------------------------------------------------------------

Thanks for the new test!  Looks like just a few small tweaks to make.  Be sure to run it on try as well.

::: browser/devtools/app-manager/test/browser_manifest_editor.js
@@ +171,5 @@
> +    let parentElem = gManifestWindow.document
> +                     .querySelector("[id ^= '" + parent + "']");
> +    ok(parentElem, "Found parent element");
> +
> +    if(parentElem == null)

The block seems unnecessary, since you are testing value above.  If it's null, the test fails anyway, which is what we want.

@@ +178,5 @@
> +    let keyExists = key in gManifestEditor.manifest[parent];
> +    ok(keyExists,
> +       "The manifest contains the key under the expected parent");
> +
> +    if(!keyExists)

Same here, remove this block.

@@ +186,5 @@
> +    let removePropertyButton = newElem.querySelector(".variables-view-delete");
> +    ok(removePropertyButton, "The remove property button was found");
> +    removePropertyButton.click();
> +
> +    waitForUpdate();

This should be "yield waitForUpdate()" as in the other functions above.

Comment 5

[Mihaela Velimiroviciu (:mihaelav)]

Attachment: updated based on review

remote:   https://tbpl.mozilla.org/?tree=Try&rev=b4e9a9752ec2

Comment 6

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

Comment on attachment 8391105 [details] [diff] [review]
updated based on review

Review of attachment 8391105 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good, thanks!

Comment 7

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/fx-team/rev/c133f3472fc0

Comment 8

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/c133f3472fc0