Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Refactor root package tests #329

Open
chrisccoulson opened this issue Sep 9, 2024 · 0 comments
Open

Refactor root package tests #329

chrisccoulson opened this issue Sep 9, 2024 · 0 comments

Comments

@chrisccoulson
Copy link
Collaborator

I want to refactor the testing to be more in line with one of the recently added packages, eg, plainkey:

  1. For New* functions, test the returned private values against statically compiled values.
  2. For serialization tests, test the serialization against a statically contained value, such as a JSON string or sequence of bytes
  3. For deserialization tests, test the deserialization against statically contained values to detect for backwards compatibility issues
  4. Add unit tests for other features as usual.
chrisccoulson added a commit to chrisccoulson/secboot that referenced this issue Jan 22, 2025
@chrisccoulson
Fix plumbing of the role via the PlatformKeyDataHandler interface
dadb3b6 
PR canonical#361 addressed the issue that most tpm2 tests were being performed
with an empty role, which isn't realistic. It turns out that I missed a
few places, particularly in some of the platform tests, that if I had
fixed in the previous PR, would have noticed that the role parameter
isn't being passed through the platform interface (this would get picked
up when changing a password because key validate would fail).

This addresses this, and fixes tests in the top-level package to catch
it as well.

Note that I'm using the same value in most tests - my normal approach to
unit testing is to test each input with at least more than one value,
but the tests in the top-level secboot package and the tpm2 sub-package
are getting a bit complicated with a lot of inconsistencies in their
approach to testing. I have issues canonical#329 and canonical#330 open to refactor the
tests so they are structured in a more consistent way, like the tests
for the recently added plainkey platform are.
@chrisccoulson chrisccoulson mentioned this issue Jan 22, 2025
Merged
chrisccoulson added a commit that referenced this issue Jan 27, 2025
@chrisccoulson
Merge pull request #366 from chrisccoulson/fix-role-plumbing
e908152 
Fix plumbing of the role via the PlatformKeyDataHandler interface.

PR #361 addressed the issue that most tpm2 tests were being performed with an empty role, which isn't realistic. It turns out that I missed a few places, particularly in some of the platform tests, that if I had fixed in the previous PR, would have noticed that the role parameter isn't being passed through the platform interface (this would get picked up when changing a password because key validation would fail).

This addresses this, and fixes tests in the top-level package to catch it as well.

Note that I'm using the same value in most tests - my normal approach to unit testing is to test each input with at least more than one value, but the tests in the top-level secboot package and the tpm2 sub-package are getting a bit complicated with a lot of inconsistencies in their approach to testing. I have issues #329 and #330 open to refactor the tests so they are structured in a more consistent way, like the tests for the recently added plainkey platform are.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
1 participant
@chrisccoulson