Skip to content

test:enhance certificate chain builder and mock build #2615

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

Merged
merged 4 commits into from
Jul 1, 2025

Conversation

Alenar
Copy link
Collaborator

@Alenar Alenar commented Jun 27, 2025

Content

This PR enhance the certificate chain builder and add propose a tooling to create mock in a standardized way.

Changes

  • Rework CertificateChainBuilder:
    • Now .build() return a CertificateChainFixture that's contains the same data as the previous returned tuple (the chain ordered from latest to genesis and the genesis verifier)
    • The addition of CertificateChainFixture allow to attach utility methods related to the produced certificates:
      • genesis_certificate(&self): shortcut to get the generated genesis
      • chain_reversed: return a copy of the generated chain reversed
      • certificate_path_to_genesis: return a list with the given certificate hash and all it's previous certificates, useful when testing certificate chain validation
    • Additionally CertificateChainFixture implement:
      • Deref and DerefMut to [Certificate] allowing to avoid directly operating on the stored certificates_chained list most of the time.
      • impl<C: From<Certificate>> From<CertificateChainFixture> for Vec<C> to allow conversion of the full list to a list of another type that can be converted from Certificate (mostly useful in db layer when handling CertificateRecord)
  • Add mock_extensions module with a MockBuilder tool that allow to create mock instance with a block dedicated to the mock configuration, avoiding pollution of the test code, exemple:
let mock = MockBuilder::<MockType>::configure(|mock| {
    mock.expect_foo().returning(|| Bar::new());
});

Pre-submit checklist

  • Branch
    • Tests are provided (if possible)
    • Crates versions are updated (if relevant)
    • Commit sequence broadly makes sense
    • Key commits have useful messages
  • PR
    • All check jobs of the CI have succeeded
    • Self-reviewed the diff
    • Useful pull request description
    • Reviewer requested
  • Documentation
    • No new TODOs introduced

Issue(s)

Relates to #2534

@Alenar Alenar self-assigned this Jun 27, 2025
@Alenar Alenar added the testing 🔁 Something related to tests label Jun 27, 2025
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR enhances the certificate chain builder by returning a dedicated CertificateChainFixture and introduces a new mock_extensions module with a MockBuilder tool for improved test configuration. Key changes include the refactoring of CertificateChainBuilder::build to return a CertificateChainFixture (with added utility methods) and updates to tests and related modules to work with this new fixture.

Reviewed Changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
mithril-common/src/test_utils/mod.rs Added module export for mock_extensions and updated re-export of certificate chain items.
mithril-common/src/test_utils/mock_extensions.rs New module providing a MockBuilder tool for easier and isolated mock configuration.
mithril-common/src/test_utils/certificate_chain_builder.rs Refactored CertificateChainBuilder to return CertificateChainFixture; added utility methods.
mithril-common/src/crypto_helper/tests_setup.rs Updated setup_certificate_chain to match the new CertificateChainFixture signature.
mithril-common/src/certificate_chain/certificate_verifier.rs Updated tests to use the new CertificateChainFixture instead of tuple-based certificate chain.
mithril-client/src/certificate_client/verify.rs Adjusted tests to use the fixture’s fields (certificates_chained and genesis_verifier).
mithril-aggregator/* Updated repository, record, and query modules to work with CertificateChainFixture accordingly.

Copy link

github-actions bot commented Jun 27, 2025

Test Results

    3 files  ± 0    102 suites  ±0   17m 19s ⏱️ +26s
2 053 tests + 5  2 053 ✅ + 5  0 💤 ±0  0 ❌ ±0 
4 244 runs  +15  4 244 ✅ +15  0 💤 ±0  0 ❌ ±0 

Results for commit 4ec743e. ± Comparison against base commit 546b1a1.

♻️ This comment has been updated with latest results.

@Alenar Alenar force-pushed the djo/test/enhance-chain_builder-and-mock_build branch 2 times, most recently from 1c3eced to 22b4bd4 Compare June 27, 2025 16:22
@Alenar Alenar temporarily deployed to testing-preview June 27, 2025 16:39 — with GitHub Actions Inactive
Copy link
Collaborator

@dlachaume dlachaume left a comment

Choose a reason for hiding this comment

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

LGTM 🚀

Copy link
Member

@jpraynaud jpraynaud left a comment

Choose a reason for hiding this comment

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

LGTM

Alenar added 3 commits June 30, 2025 17:20
Instead of a tuple, this allow to extend the returned value with usefull
fstructeatures for test (i.e. immediate access to genesis certificate).

Also update the builder so generated certificates have different,
deterministic, signed entity type.
@Alenar Alenar force-pushed the djo/test/enhance-chain_builder-and-mock_build branch from 22b4bd4 to 89a6781 Compare June 30, 2025 16:00
* mithril-aggregator from `0.7.68` to `0.7.69`
* mithril-client from `0.12.17` to `0.12.18`
* mithril-common from `0.6.2` to `0.6.3`
@Alenar Alenar temporarily deployed to testing-preview June 30, 2025 16:14 — with GitHub Actions Inactive
@Alenar Alenar merged commit 06a9beb into main Jul 1, 2025
66 of 68 checks passed
@Alenar Alenar deleted the djo/test/enhance-chain_builder-and-mock_build branch July 1, 2025 07:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
testing 🔁 Something related to tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants