-
Notifications
You must be signed in to change notification settings - Fork 14
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.
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
dcap all sgx statuses #225
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some minor things, but looks good to me in general.
#[cfg(test)] | ||
println!("certificate chain is invalid: {:?}", e); | ||
Error::CertificateChainIsInvalid |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Don't we want to go with the approach you chose in the last pr and simply make this trace?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I couldn't make it work with cargo test -- --nocapture. that's why I used println specifically here, to see the inner error which we swallow otherwise
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You need to initialize the (env) logger in each test to make this work.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@clangenb if you can live with my responses, please approve and resolve the comments. or insist ;-) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, there would be a solution to the logger issue, but I can live with it for now.
closes #217
added one unit test to check outdated status. we could certainly be more thorough with unit testing all possible combinations of tcb info with various quotes. But I guess for now this does test the new feature minimally