Move some test from unit test to integration tests#414
Open
simo5 wants to merge 5 commits intolatchset:mainfrom
Open
Move some test from unit test to integration tests#414simo5 wants to merge 5 commits intolatchset:mainfrom
simo5 wants to merge 5 commits intolatchset:mainfrom
Conversation
The `test_login` unit test was causing intermittent failures due to its sensitivity to the execution order of threads and potential interference from other tests running in the same process. This change converts the unit test into an integration test, which runs in a separate process. This isolation eliminates the source of the flakiness, making the test more reliable. The test has also been rewritten to use the higher-level `cryptoki` crate instead of the raw C FFI calls. Co-authored-by: Gemini <gemini@google.com> Signed-off-by: Simo Sorce <simo@redhat.com>
simo5
changed the title
simo5
force-pushed
the
integr_tests
branch
6 times, most recently
from
dca1287 to
9114e58
Compare
A new `rc_common.rs` module is introduced to centralize the setup logic for integration tests. The `setup_token` function encapsulates the process of initializing the PKCS#11 library, creating a clean token for testing, and setting up the SO and User PINs. This change removes significant code duplication from the test files, making them cleaner and easier to maintain. Co-authored-by: Gemini <gemini@google.com> Signed-off-by: Simo Sorce <simo@redhat.com>
Jakuje
reviewed
Feb 6, 2026
Improve the rust-cryptoki integration test infrastructure to provide better test isolation and configuration management. Each test now runs in its own temporary directory with a dedicated configuration file. This change introduces new `setup` and `modify_setup` helpers that allow tests to dynamically create and change the configuration, then re-initialize the PKCS#11 module to pick up the changes. To support this, the core `C_Initialize` and `C_Finalize` functions have been updated. The configuration is now reloaded on `C_Initialize` and cleared on `C_Finalize`, allowing for proper re-initialization within the same process. A new integration test is added to verify the `ec_point_encoding` compatibility setting for EdDSA keys, replacing an older, less robust internal test. Co-authored-by: Gemini <gemini@google.com> Signed-off-by: Simo Sorce <simo@redhat.com>
The test setup helper functions in `rc_common` are refactored to simplify testing against multiple database backends. The `setup_token` function now dynamically generates the configuration required for sqlite, nssdb, and memory backends. This allows for more comprehensive integration testing. A new test is added to verify that token state is correctly preserved after a `C_Finalize` and `C_Initialize` sequence. This new, higher-level test replaces the previous C-style re-initialization tests. Existing tests are updated to use the new, unified setup function. Co-authored-by: Gemini <gemini@google.com> Signed-off-by: Simo Sorce <simo@redhat.com>
Extract common logic within the `rc_common.rs` test helper module into dedicated functions. The logic for determining the PKCS#11 module path and the temporary directory is now in `get_module()` and `get_tmpdir()`. The configuration file creation logic, previously duplicated across setup functions, is centralized in a new `generate_config` function. This change reduces code duplication and improves the readability and maintainability of the test code. Co-authored-by: Gemini <gemini@google.com> Signed-off-by: Simo Sorce <simo@redhat.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
The
test_loginunit test was causing intermittent failures due to its sensitivity to the execution order of threads and potential interference from other tests running in the same process.This change converts the unit test into an integration test, which runs in a separate process. This isolation eliminates the source of the flakiness, making the test more reliable. The test has also been rewritten to use the higher-level
cryptokicrate instead of the raw C FFI calls.Additionally expand the integration tests with more common code and move the eddsa test that require configuration changes also to an integration test.
This unvelied that fn_finalize followed from fo initialize was not re-reading the configuration so adjusted also that part to do the sensible thing.
Multiple fn_initialize calls will still not cause the config to be reread, only if no config was available fn_initialize causes a re-read and fn_finalize now throws away the config so it can be re-parsed.
Finally,
this PR also addressed a nasty issue that was unveiled when changing how fn_initialize/fn_finalize behavior that evaded testing for quite some time because the code was explicitly behaving in different way in test vs build cases, thanks to Jakub for adding a nasty test in CI that uncovered this eventually :)
Fixes #409
Checklist
Reviewer's checklist: