1077330 - [Wifi] Add marionette test for Wifi certificate functions.

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Wifi, defect)

Description

[Chuck Lee [:chucklee, Triggered by NI, Review, and Feedback requests]]

The test should include import server certificate, import user certificate, import pkcs12 certificate, list imported certificates, and delete imported certificates.

Comment 1

[Chuck Lee [:chucklee, Triggered by NI, Review, and Feedback requests]]

Attachment: WIP - Marionette test for wifi certificate API.

Do certificate function test by import, list, then delete certificate of different types.

Comment 2

[Henry Chang [:hchang]]

Comment on attachment 8500892 [details] [diff] [review]
WIP - Marionette test for wifi certificate API.

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

Looks neat and good to me! However, test flow seems similar in all test cases. 
Is there any chance to have |doTestCert| which takes a testCertInfo as argument
to remove the duplicated code in all test cases?

Comment 3

[Chuck Lee [:chucklee, Triggered by NI, Review, and Feedback requests]]

Attachment: Marionette test for wifi certificate API.

Address comment 2.

Comment 4

[Vincent Chang[:vchang][changyihsin]]

Comment on attachment 8503958 [details] [diff] [review]
Marionette test for wifi certificate API.

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

I think you attach the wrong patches.

Comment 5

[Chuck Lee [:chucklee, Triggered by NI, Review, and Feedback requests]]

Attachment: Marionette test for wifi certificate API.

Attach correct patch, thanks Vincent.

Comment 6

[Henry Chang [:hchang]]

Comment on attachment 8505177 [details] [diff] [review]
Marionette test for wifi certificate API.

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

Nothing incorrect to me! Thanks :) You may consider to remove one use of if-else
as I comment. 

By the way, these test cases verify the certificate import/delete. Is the stock 
goldfish hostapd able to support using that certificate?

::: dom/wifi/test/marionette/head.js
@@ +1389,5 @@
> +          return aTestCaseChain().then(() => deleteCert(nickname));
> +        } else {
> +          return deleteCert(nickname);
> +        }
> +      })

It could be written more clearly as the following:

.then(function(list) {
  for (let i = 0; i < usage.length; i++) {
    isnot(list[usage[i]].indexOf(nickname), -1,
          "Certificate \"" + nickname + "\" of usage " + usage[i] + " is imported");
  }
})
.then(aTestCaseChain)
.then(() => deleteCert(nickname))

Comment 7

[Vincent Chang[:vchang][changyihsin]]

Comment on attachment 8505177 [details] [diff] [review]
Marionette test for wifi certificate API.

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

Good job !!! I think we are going to have test cases for connecting to EAP-xxx type APs soon.

Comment 8

[Chuck Lee [:chucklee, Triggered by NI, Review, and Feedback requests]]

(In reply to Henry Chang [:henry] from comment #6)
> Comment on attachment 8505177 [details] [diff] [review]
> Marionette test for wifi certificate API.
> 
> Review of attachment 8505177 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Nothing incorrect to me! Thanks :) You may consider to remove one use of
> if-else
> as I comment. 
> 
> By the way, these test cases verify the certificate import/delete. Is the
> stock 
> goldfish hostapd able to support using that certificate?
> 

Imported certificates can be accessed by gecko and wpa_supplicant.
Also server requires different set of certificate, I am checking if hostapd on emulator has internal EAP server and how to set it up if it supports.

Comment 9

[Chuck Lee [:chucklee, Triggered by NI, Review, and Feedback requests]]

Attachment: Marionette test for wifi certificate API. V2. r=hchang, vchang

Address comment 6 and add reviewers' names.

Comment 10

[Chuck Lee [:chucklee, Triggered by NI, Review, and Feedback requests]]

Still trying to figure out why this patch keeps busting mochitest B2G ICS Emulator opt M9: https://treeherder.mozilla.org/#/jobs?repo=try&revision=6acea0825ebb

Comment 11

[Chuck Lee [:chucklee, Triggered by NI, Review, and Feedback requests]]

Try works after retry without modifying patch: https://treeherder.mozilla.org/#/jobs?repo=try&revision=2bb4304e699c
Added marionette tests(test_wifi_manage_server_certificate.js, test_wifi_manage_user_certificate.js, test_wifi_manage_pkcs12_certificate.js) are passed: http://ftp.mozilla.org/pub/mozilla.org/b2g/try-builds/[e-mail]-2bb4304e699c/try-emulator/try_ubuntu64_vm-b2g-emulator_test-marionette-webapi-bm123-tests1-linux64-build52.txt.gz

Comment 12

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/b2g-inbound/rev/1af5abd672d3

Comment 13

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/1af5abd672d3