Skip to content
This repository has been archived by the owner on Jun 3, 2020. It is now read-only.

Add integration tests for yubihsm subcommands #121

Merged
merged 3 commits into from
Nov 26, 2018
Merged

Add integration tests for yubihsm subcommands #121

merged 3 commits into from
Nov 26, 2018

Conversation

liamsi
Copy link
Contributor

@liamsi liamsi commented Nov 26, 2018

ref #114

- import subcommand takes optional kms config path
@liamsi liamsi force-pushed the test_commands branch 2 times, most recently from 697391a to 3ddba11 Compare November 26, 2018 16:33
@liamsi liamsi changed the title WIP: tests for yubihsm subcommands Add integration tests for yubihsm subcommands Nov 26, 2018
@tarcieri
Copy link
Contributor

@liamsi FWIW, I'm ok with using 1.30+

@liamsi
Copy link
Contributor Author

liamsi commented Nov 26, 2018

Cool, should we update the docker file then? Should I create a new one?

@tarcieri
Copy link
Contributor

@liamsi yeah, I'd suggest updating the Dockerfile (maybe in a separate PR, as this seems to be OK for now)

String::from_utf8(out.stderr)
.unwrap()
.trim()
.starts_with("error: no keys in this YubiHSM")
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We can probably add more realistic integration tests too.
sth along the lines of:
list -> generate -> list -> import -> list

Copy link
Contributor

@tarcieri tarcieri Nov 26, 2018

Choose a reason for hiding this comment

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

The main problem with that sort of thing right now is the MockHSM does not persist its state between invocations of tmkms.

I could look into some way to do that though (if nothing else just making its internal state Serializeable and dumping it to a JSON file or something)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think that would be neat. But I also think it's probably not a high priority I guess.

Copy link
Contributor

@tarcieri tarcieri left a comment

Choose a reason for hiding this comment

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

Awesome

@liamsi liamsi merged commit 2e85505 into master Nov 26, 2018
@liamsi liamsi deleted the test_commands branch November 26, 2018 18:06
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants