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

Make it possible to implement integration tests #166

Closed
sosthene-nitrokey opened this issue Jan 4, 2024 · 9 comments
Closed

Make it possible to implement integration tests #166

sosthene-nitrokey opened this issue Jan 4, 2024 · 9 comments

Comments

@sosthene-nitrokey
Copy link
Contributor

Since the module is a cdylib it is not possible to have integration tests on the main crate.

I suggest splitting the crate into a normal rust crate and a cdylib that just reexports it that way integration testing is possible.

@robin-nitrokey
Copy link
Member

What kind of integration tests would you like to add?

Generally, I think it would make sense to split the code into the unsafe C API that just does the data mapping, and a safe Rust API that returns Result<_, CK_RV>. That would also make the code much more ergonomic. Potentially, macros could be used to reduce the amount of boilerplate code. (See also: #138)

@sosthene-nitrokey
Copy link
Contributor Author

I wanted to add tests that used different config files to test #165 on bad files, fingerprints etc... I can't do that in the normal tests because the env::set_var is global and the device is only initialized once, so I need a specific test executable for each test.

@robin-nitrokey
Copy link
Member

Maybe the change to C_Initialize/C_Finalize I suggested in that PR would already be enough for testing that? I’m not opposed to splitting the crate, but as it adds additional release and maintenance effort, I would first try out simpler solutions.

@sosthene-nitrokey
Copy link
Contributor Author

That would require serializing all the tests.

@robin-nitrokey
Copy link
Member

Would that be an issue? I think if we really want integration tests for a library with a global state, we need sequential tests. If you only want to test the init implementation without the PKCS11 wrapper, you could also test read_configuration directly (and move the env access out of that function).

@sosthene-nitrokey
Copy link
Contributor Author

I would want to test the full DEVICE initialization. But that can probably be done that way indeed.

@robin-nitrokey
Copy link
Member

You will probably run into issues with repeated calls to log::set_logger if you also test applying the configuration.

@sosthene-nitrokey
Copy link
Contributor Author

I was able to implement this in 4157673

@sosthene-nitrokey
Copy link
Contributor Author

Some form of integration tests have been implemented as part of #214

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

No branches or pull requests

2 participants