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.

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

Add first set of integration tests #54

Closed
wants to merge 1 commit into from
Closed

Add first set of integration tests #54

wants to merge 1 commit into from

Conversation

d-e-s-o
Copy link
Owner

@d-e-s-o d-e-s-o commented Jan 7, 2019

This change introduces the first set of integration-style test for the
application. Those tests may or may not connect to an actual Nitrokey
device (depending on what they test). We use the nitrokey-test crate's
test attribute macro to automatically dispatch tests to connected
devices or skip them if a required device is not present. It also
provides the means for automatically serializing tests.

This change introduces the first set of integration-style test for the
application. Those tests may or may not connect to an actual Nitrokey
device (depending on what they test). We use the nitrokey-test crate's
test attribute macro to automatically dispatch tests to connected
devices or skip them if a required device is not present. It also
provides the means for automatically serializing tests.
@d-e-s-o d-e-s-o self-assigned this Jan 7, 2019
@d-e-s-o
Copy link
Owner Author

d-e-s-o commented Jan 7, 2019

@robinkrahl , would be nice if you could take a look and let me know what you think. Do you think the test organization makes sense? I definitely think that we should not put everything into a single file (as we did for commands.rs). I envisioned splitting by command (only done for status right now), with run.rs containing some more or less generic tests pertaining the execution.

@d-e-s-o
Copy link
Owner Author

d-e-s-o commented Jan 7, 2019

If you feel that the patch is too big I can split it. I felt that a reasonable amount is boilerplate that does not need a close review.

@robinkrahl
Copy link
Collaborator

I’ll have a look at it! Regarding code organization, I think we could also consider splitting commands.rs into submodules (per subcommand).

@d-e-s-o
Copy link
Owner Author

d-e-s-o commented Jan 7, 2019

Thanks!

Regarding code organization, I think we could also consider splitting commands.rs into submodules (per subcommand).

Yeah, I am looking into preparing a patch that removes some of the command enum related boilerplate (basically auto generating it). Let's see how bad it is after that. I believe there was a reason for the organization, but I don't recall exactly. I may just remember wrongly.

@d-e-s-o
Copy link
Owner Author

d-e-s-o commented Jan 7, 2019

Thank you :)

@d-e-s-o d-e-s-o closed this Jan 7, 2019
@d-e-s-o d-e-s-o deleted the topic/tests branch January 7, 2019 18:57
@d-e-s-o d-e-s-o mentioned this pull request Jan 7, 2019
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.

2 participants