Implemented and Tested GetRandom#277
Conversation
chrisfenner
left a comment
chrisfenner
left a comment
There was a problem hiding this comment.
Looks great, thanks for the change! I have two tiny nits.
| @@ -0,0 +1,24 @@ | |||
| package tpm2 | |||
There was a problem hiding this comment.
(not a blocking comment) how do we want to organize tests? having a test per command may make the directory quite large. we could have a test directory like in tpm2 or we could split by use case.
There was a problem hiding this comment.
My mild opinion is that I like having more and smaller test files. A test directory with e.g., one file per command tested would be groovy.
There was a problem hiding this comment.
I generally agree this makes tests easier to find, and we should go for this. I do wonder about tests that use multiple commands like a potential policy auth test with multiple policies.
There was a problem hiding this comment.
Right, and we'd have a lot of helpers too. So we might have some per-command tests as well as some "integration-y" tests that really cover a bunch of commands. That's a good reason not to be rigid on "one file per command" but in general I think it's ok to have a lot of files.
|
LGTM - Matt can you update your branch with a rebase? |
matt-tsai
force-pushed
the
getRandomBranch
branch
from
ed55b4e to
5755204
Compare
* Implement and tested go-tpm direct function TPM2_GetRandom * Resolved all nits from PR Co-authored-by: Matthew Tsai <matthewtsai@google.com>
matt-tsai
changed the title
matt-tsai
changed the title
3 participants
No description provided.