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

Simplify aggregator integration tests #1003

Merged
merged 7 commits into from Jun 26, 2023

Conversation

Alenar
Copy link
Collaborator

@Alenar Alenar commented Jun 22, 2023

Content

This PR simplify the integration tests of the aggregator by replacing most asserts by a new macro assert_last_certificate_eq.
Before we to manually get multiple information in the tested aggregator to do our assertion, the new mactro streamlined this and a new type, ExpectedCertificate, allow to specify the subset of data that is meaningful to verify in a Certificate.

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
    • No clippy warnings in the CI
    • Self-reviewed the diff
    • Useful pull request description
    • Reviewer requested

Comments

To make the new macro work the capacity to get a SignedEntity by its Certificate id had to be added to the SignedEntityStorer.

@Alenar Alenar force-pushed the djo/simplify_aggregator_integration_tests branch from d7bf13a to 04111cc Compare June 23, 2023 07:57
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 👍

mithril-aggregator/tests/certificate_chain.rs Outdated Show resolved Hide resolved
Right now it's now only used by tests to assert that a given signed entity
was created with the last certificate.
This simplify test by shifting the responsability of getting the data to
check to the tests toolings, also it makes more visual what we expect
making reading those tests less cumbersome.
@Alenar Alenar force-pushed the djo/simplify_aggregator_integration_tests branch from 04111cc to 6f692cc Compare June 26, 2023 10:17
@Alenar Alenar force-pushed the djo/simplify_aggregator_integration_tests branch from 6f692cc to 4e2ad16 Compare June 26, 2023 12:43
@Alenar Alenar temporarily deployed to testing-preview June 26, 2023 12:53 — with GitHub Actions Inactive
@Alenar Alenar merged commit a259add into main Jun 26, 2023
26 of 50 checks passed
@Alenar Alenar deleted the djo/simplify_aggregator_integration_tests branch June 26, 2023 12:55
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.

None yet

2 participants