Skip to content

Fix failing tests#268

Merged
allisonmoyer merged 2 commits into
masterfrom
amoyer/failing-tests
Jun 7, 2021
Merged

Fix failing tests#268
allisonmoyer merged 2 commits into
masterfrom
amoyer/failing-tests

Conversation

@allisonmoyer

Copy link
Copy Markdown
Collaborator

No description provided.

@codecov-commenter

Copy link
Copy Markdown

Codecov Report

Merging #268 (dfe7f66) into master (a8bbee4) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #268   +/-   ##
=======================================
  Coverage   86.60%   86.60%           
=======================================
  Files          16       16           
  Lines        1053     1053           
=======================================
  Hits          912      912           
  Misses        141      141           

@dfed dfed left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, but how was this test failing before? Def seems like a copy/paste issue but since we were testing that these were different I'd expect the tests to have been succeeding.


try valet().setString(passcode, forKey: key)
let equivalentValet = SecureEnclaveValet.valet(with: valet().identifier, accessControl: .devicePasscode)
let equivalentValet = SinglePromptSecureEnclaveValet.valet(with: valet().identifier, accessControl: .devicePasscode)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This makes sense. Though maybe let's rename equivalentValet to similarValet since they aren't actually equivalent?

@allisonmoyer

Copy link
Copy Markdown
Collaborator Author

LGTM, but how was this test failing before? Def seems like a copy/paste issue but since we were testing that these were different I'd expect the tests to have been succeeding.

I'm not sure. There's more that are failing locally for me as well (like test_secureEnclaveValetsWithDifferingAccessControl_canNotAccessSameData and all of the ValetBackwardsCompatibilityIntegrationTests on macOS), so I was trying to trigger the test suite to see if my local failures would reflect on CI, but looks like they don't

@dfed

dfed commented Jun 7, 2021

Copy link
Copy Markdown
Collaborator

On target are the tests failing? If it's macOS there's likely a code-signing issue. Check out these instructions.

@allisonmoyer allisonmoyer marked this pull request as ready for review June 7, 2021 19:21
@allisonmoyer allisonmoyer merged commit a1b2e46 into master Jun 7, 2021
@dfed dfed deleted the amoyer/failing-tests branch December 14, 2022 20:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants