New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 827677 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Last visit > 30 days ago
Closed: Apr 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Android , Chrome , Mac
Pri: 2
Type: Bug

Blocking:
issue 780078


Participants' hotlists:
Hotlist-1


Sign in to add a comment

Optimize unit test using FakeFidoRequestHandler for readability

Project Member Reported by hongjunchoi@chromium.org, Mar 30 2018

Issue description

Currently, FakeFidoRequestHandler tests failure based on whether mocked device returns 1) base::nullopt(processing error) , 2) empty vector (UP-verified error), 3) non-empty vector(success). 

Change this logic for readability.  
 

Comment 1 by engedy@chromium.org, Mar 31 2018

Labels: Hotlist-WebAuthnFixit
Owner: ----
Status: Available (was: Assigned)
For methods taking two arguments such as:

  CtapDeviceResponseCode return_code,
  base::Optional<AuthenticatorMakeCredentialResponse> response_data

We should probably just:

  DCHECK_EQ(
    return_code == CtapDeviceResponseCode::kSuccess,
    response_data.has_value()); 

Comment 2 by engedy@chromium.org, Mar 31 2018

Labels: -Pri-3 M-67 Pri-2
Owner: hongjunchoi@chromium.org
Status: Started (was: Available)
Project Member

Comment 4 by bugdroid1@chromium.org, Apr 12 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/0cfdea8931d2aa8a3ec5414435c8652324f0027e

commit 0cfdea8931d2aa8a3ec5414435c8652324f0027e
Author: Jun Choi <hongjunchoi@chromium.org>
Date: Thu Apr 12 15:20:16 2018

Clean FakeFidoTask response handling

Currenly FakeFidoTask used in unit test for FidoRequestHandler
categorizes response types depending on whether the response received
from the mock device is base::nullopt, empty vector, or a non-empty
vector. Change this categorization to use clearly readable enums instead.

Bug:  827677 
Change-Id: I6817808a6ace85e88bba48039c9cbe054cfbfb81
Reviewed-on: https://chromium-review.googlesource.com/1006233
Commit-Queue: Jun Choi <hongjunchoi@chromium.org>
Reviewed-by: Jan Wilken Dörrie <jdoerrie@chromium.org>
Cr-Commit-Position: refs/heads/master@{#550208}
[modify] https://crrev.com/0cfdea8931d2aa8a3ec5414435c8652324f0027e/device/fido/fido_request_handler_unittest.cc

Status: Fixed (was: Started)
Project Member

Comment 6 by bugdroid1@chromium.org, Apr 17 2018

Labels: merge-merged-testbranch
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/0cfdea8931d2aa8a3ec5414435c8652324f0027e

commit 0cfdea8931d2aa8a3ec5414435c8652324f0027e
Author: Jun Choi <hongjunchoi@chromium.org>
Date: Thu Apr 12 15:20:16 2018

Clean FakeFidoTask response handling

Currenly FakeFidoTask used in unit test for FidoRequestHandler
categorizes response types depending on whether the response received
from the mock device is base::nullopt, empty vector, or a non-empty
vector. Change this categorization to use clearly readable enums instead.

Bug:  827677 
Change-Id: I6817808a6ace85e88bba48039c9cbe054cfbfb81
Reviewed-on: https://chromium-review.googlesource.com/1006233
Commit-Queue: Jun Choi <hongjunchoi@chromium.org>
Reviewed-by: Jan Wilken Dörrie <jdoerrie@chromium.org>
Cr-Commit-Position: refs/heads/master@{#550208}
[modify] https://crrev.com/0cfdea8931d2aa8a3ec5414435c8652324f0027e/device/fido/fido_request_handler_unittest.cc

Sign in to add a comment