Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2873973004:
Implement the sign-in screen test app for manual testing (Closed)
DescriptionImplement the sign-in screen test app for manual testing
The test app allows to perform the following manual testing when
installed through the DeviceLoginScreenAppInstallList device policy into
the Chrome OS sign-in profile:
* Click "Add user" and type the username on the test domain;
* The certificate selection dialog pops out, containing the certificate
provided by the test app;
* After selecting the certificate, the PIN dialog appears;
* Wrongly typed PIN results in keeping the dialog displayed, but now
with an additional error message;
* Correctly typed PIN hides the PIN dialog, and moves the login process
forward;
* Afterwards, the login process is expected to fail, as the test app
doesn't implement the real signing operation and responds with a
garbage data.
BUG=626343
TEST=manual - install the app through the device policy via YAPS and
follow the test steps described above
TBR=achuith@
Review-Url: https://codereview.chromium.org/2873973004
Cr-Commit-Position: refs/heads/master@{#471114}
Committed: https://chromium.googlesource.com/chromium/src/+/a9ad5af20477742a1d287dfb9a7fbbfd63354082
Patch Set 1 #
Total comments: 10
Patch Set 2 : Fix nits #
Messages
Total messages: 20 (11 generated)
Description was changed from
==========
Implement the sign-in screen test app for manual testing
BUG=626343
==========
to
==========
Implement the sign-in screen test app for manual testing
The test app allows to perform the following manual testing when
installed through the DeviceLoginScreenAppInstallList device policy into
the Chrome OS sign-in profile:
* Click "Add user" and type the username on the test domain;
* The certificate selection dialog pops out, containing the certificate
provided by the test app;
* After selecting the certificate, the PIN dialog appears;
* Wrongly typed PIN results in keeping the dialog displayed, but now
with an additional error message;
* Correctly typed PIN hides the PIN dialog, and moves the login process
forward;
* Afterwards, the login process is expected to fail, as the test app
doesn't implement the real signing operation and responds with a
garbage data.
BUG=626343
TEST=manual - install the app through the device policy via YAPS and
follow the test steps described above
==========
emaxx@chromium.org changed reviewers: + pmarko@chromium.org
Pavol, PTAL. Note that this is just an app for performing the manual testing; it's not intended to be used in production or (currently) in the automated tests. Let's consider this as a step of your ramping up onto the "apps on the login screen" project :)
LGTM with very small nits/suggestions. https://codereview.chromium.org/2873973004/diff/1/chrome/test/data/extensions... File chrome/test/data/extensions/signin_screen_test_app/app/background.js (right): https://codereview.chromium.org/2873973004/diff/1/chrome/test/data/extensions... chrome/test/data/extensions/signin_screen_test_app/app/background.js:12: Consider adding a short comment here that debug output is on the console and remote debugging is expected to read it because of UI limitations of apps running on the sign-in screen. https://codereview.chromium.org/2873973004/diff/1/chrome/test/data/extensions... chrome/test/data/extensions/signin_screen_test_app/app/background.js:163: var TEST_CERTIFICATE_SUPPORTED_HASHES = Should we refer to chrome/common/extensions/api/certificate_provider.idl in a comment here or is that excessive? https://codereview.chromium.org/2873973004/diff/1/chrome/test/data/extensions... chrome/test/data/extensions/signin_screen_test_app/app/background.js:226: dumpCertificateInfos(rejectedCertificates)); nit: This seems to be an array of the certificates, not of CertificateInfos [1][2]. Not that it mattered while we're only providing valid certs :) [1] https://cs.chromium.org/chromium/src/chrome/common/extensions/api/certificate... [2] https://cs.chromium.org/chromium/src/chrome/browser/extensions/api/certificat... https://codereview.chromium.org/2873973004/diff/1/chrome/test/data/extensions... chrome/test/data/extensions/signin_screen_test_app/app/background.js:295: // Stops the PIN request: if there is no error passed, the PIN dialog is closed; nit: if no error is passed (null) https://codereview.chromium.org/2873973004/diff/1/chrome/test/data/extensions... chrome/test/data/extensions/signin_screen_test_app/app/background.js:296: // if there is some, then it will be displayed in the inputless dialog. What does inputless mean here?
https://codereview.chromium.org/2873973004/diff/1/chrome/test/data/extensions... File chrome/test/data/extensions/signin_screen_test_app/app/background.js (right): https://codereview.chromium.org/2873973004/diff/1/chrome/test/data/extensions... chrome/test/data/extensions/signin_screen_test_app/app/background.js:12: On 2017/05/11 18:24:43, pmarko wrote: > Consider adding a short comment here that debug output is on the console and > remote debugging is expected to read it because of UI limitations of apps > running on the sign-in screen. Done. https://codereview.chromium.org/2873973004/diff/1/chrome/test/data/extensions... chrome/test/data/extensions/signin_screen_test_app/app/background.js:163: var TEST_CERTIFICATE_SUPPORTED_HASHES = On 2017/05/11 18:24:42, pmarko wrote: > Should we refer to chrome/common/extensions/api/certificate_provider.idl in a > comment here or is that excessive? Done. Decided to refer to the public documentation though. https://codereview.chromium.org/2873973004/diff/1/chrome/test/data/extensions... chrome/test/data/extensions/signin_screen_test_app/app/background.js:226: dumpCertificateInfos(rejectedCertificates)); On 2017/05/11 18:24:43, pmarko wrote: > nit: This seems to be an array of the certificates, not of CertificateInfos > [1][2]. Not that it mattered while we're only providing valid certs :) > > [1] > https://cs.chromium.org/chromium/src/chrome/common/extensions/api/certificate... > [2] > https://cs.chromium.org/chromium/src/chrome/browser/extensions/api/certificat... Oh, good catch. Fixed. https://codereview.chromium.org/2873973004/diff/1/chrome/test/data/extensions... chrome/test/data/extensions/signin_screen_test_app/app/background.js:295: // Stops the PIN request: if there is no error passed, the PIN dialog is closed; On 2017/05/11 18:24:42, pmarko wrote: > nit: if no error is passed (null) Done. https://codereview.chromium.org/2873973004/diff/1/chrome/test/data/extensions... chrome/test/data/extensions/signin_screen_test_app/app/background.js:296: // if there is some, then it will be displayed in the inputless dialog. On 2017/05/11 18:24:43, pmarko wrote: > What does inputless mean here? Reworded this to a clearer "the dialog that allows no further input".
The CQ bit was checked by emaxx@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from pmarko@chromium.org Link to the patchset: https://codereview.chromium.org/2873973004/#ps20001 (title: "Fix nits")
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
No L-G-T-M from a valid reviewer yet. CQ run can only be started once the patch has received an L-G-T-M from a full committer. Even if an L-G-T-M may have been provided, it was from a non-committer,_not_ a full super star committer. Committers are members of the group "project-chromium-committers". Note that this has nothing to do with OWNERS files.
Description was changed from
==========
Implement the sign-in screen test app for manual testing
The test app allows to perform the following manual testing when
installed through the DeviceLoginScreenAppInstallList device policy into
the Chrome OS sign-in profile:
* Click "Add user" and type the username on the test domain;
* The certificate selection dialog pops out, containing the certificate
provided by the test app;
* After selecting the certificate, the PIN dialog appears;
* Wrongly typed PIN results in keeping the dialog displayed, but now
with an additional error message;
* Correctly typed PIN hides the PIN dialog, and moves the login process
forward;
* Afterwards, the login process is expected to fail, as the test app
doesn't implement the real signing operation and responds with a
garbage data.
BUG=626343
TEST=manual - install the app through the device policy via YAPS and
follow the test steps described above
==========
to
==========
Implement the sign-in screen test app for manual testing
The test app allows to perform the following manual testing when
installed through the DeviceLoginScreenAppInstallList device policy into
the Chrome OS sign-in profile:
* Click "Add user" and type the username on the test domain;
* The certificate selection dialog pops out, containing the certificate
provided by the test app;
* After selecting the certificate, the PIN dialog appears;
* Wrongly typed PIN results in keeping the dialog displayed, but now
with an additional error message;
* Correctly typed PIN hides the PIN dialog, and moves the login process
forward;
* Afterwards, the login process is expected to fail, as the test app
doesn't implement the real signing operation and responds with a
garbage data.
BUG=626343
TEST=manual - install the app through the device policy via YAPS and
follow the test steps described above
TBR=achuith@
==========
The CQ bit was checked by emaxx@chromium.org
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: win_chromium_x64_rel_ng on master.tryserver.chromium.win (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)
The CQ bit was checked by emaxx@chromium.org
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
CQ is committing da patch.
Bot data: {"patchset_id": 20001, "attempt_start_ts": 1494539498555740,
"parent_rev": "29c12a26e644a95e6d53eb7e02c136ca9fdf4c94", "commit_rev":
"a9ad5af20477742a1d287dfb9a7fbbfd63354082"}
Message was sent while issue was closed.
Description was changed from
==========
Implement the sign-in screen test app for manual testing
The test app allows to perform the following manual testing when
installed through the DeviceLoginScreenAppInstallList device policy into
the Chrome OS sign-in profile:
* Click "Add user" and type the username on the test domain;
* The certificate selection dialog pops out, containing the certificate
provided by the test app;
* After selecting the certificate, the PIN dialog appears;
* Wrongly typed PIN results in keeping the dialog displayed, but now
with an additional error message;
* Correctly typed PIN hides the PIN dialog, and moves the login process
forward;
* Afterwards, the login process is expected to fail, as the test app
doesn't implement the real signing operation and responds with a
garbage data.
BUG=626343
TEST=manual - install the app through the device policy via YAPS and
follow the test steps described above
TBR=achuith@
==========
to
==========
Implement the sign-in screen test app for manual testing
The test app allows to perform the following manual testing when
installed through the DeviceLoginScreenAppInstallList device policy into
the Chrome OS sign-in profile:
* Click "Add user" and type the username on the test domain;
* The certificate selection dialog pops out, containing the certificate
provided by the test app;
* After selecting the certificate, the PIN dialog appears;
* Wrongly typed PIN results in keeping the dialog displayed, but now
with an additional error message;
* Correctly typed PIN hides the PIN dialog, and moves the login process
forward;
* Afterwards, the login process is expected to fail, as the test app
doesn't implement the real signing operation and responds with a
garbage data.
BUG=626343
TEST=manual - install the app through the device policy via YAPS and
follow the test steps described above
TBR=achuith@
Review-Url: https://codereview.chromium.org/2873973004
Cr-Commit-Position: refs/heads/master@{#471114}
Committed:
https://chromium.googlesource.com/chromium/src/+/a9ad5af20477742a1d287dfb9a7f...
==========
Message was sent while issue was closed.
Committed patchset #2 (id:20001) as https://chromium.googlesource.com/chromium/src/+/a9ad5af20477742a1d287dfb9a7f... |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
