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

Itetst: Cleanup ExpectedInvocactions #469

Open
arajasek opened this issue Jun 13, 2022 · 3 comments
Open

Itetst: Cleanup ExpectedInvocactions #469

arajasek opened this issue Jun 13, 2022 · 3 comments
Labels
cleanup enhancement New feature or request good first issue Good for newcomers P3 testing

Comments

@arajasek
Copy link
Contributor

The current expectations lead to a lot of bloat in the tests. It may not be too easy to avoid this, since they need to be fairly flexible, but there's likely some helpers that can be created to simplify this.

@LesnyRumcajs
Copy link
Contributor

@arajasek @ZenGround0
I was thinking roughly of an API like this, kind of inspired by what clap provides:

ExpectInvocation::new()
    .to(CronActor)
    .method(CronMethod::EpochTick)
    .sub_invoc(vec![
        ExpectInvocation::new()
            .to(StoragePowerActor)
            .method(PowerMethod::onEpochTickEnd),
        ExpectInvocation::new()
            .to(RewardActor)
            .method(RewardMethod::UpdateNetworkKPI)
        ])
    .assert_match(v);

that would replace

    ExpectInvocation {
        to: *CRON_ACTOR_ADDR,
        method: CronMethod::EpochTick as u64,
        subinvocs: Some(vec![
            ExpectInvocation {
                to: *STORAGE_POWER_ACTOR_ADDR,
                method: PowerMethod::OnEpochTickEnd as u64,
                ..Default::default()
            },
            ExpectInvocation {
                to: *REWARD_ACTOR_ADDR,
                method: RewardMethod::UpdateNetworkKPI as u64,
                ..Default::default()
            },
        ]),
        ..Default::default()
    }
    .matches(v.take_invocations().last().unwrap());

In essence, it gets rid of manual casts in every method, having to call Default::default and whatever could be abstracted away so even a non-Rust dev could understand what is happening.

We could get even fancier with a limited amount of macros if we expect hundreds of such tests but IMO this alone would make the ITGs codebase a bit cleaner and easier to read.

@anorth anorth added enhancement New feature or request good first issue Good for newcomers labels Aug 11, 2022
@anorth anorth added testing and removed area/test labels Mar 30, 2023
@anorth
Copy link
Member

anorth commented Oct 8, 2023

integration_tests/src/expects.rs added a bunch of helper methods that in practise reduce a lot of the noise in tests. The pattern is limited by not handling sub-invocations, though.

@anorth anorth added the cleanup label May 27, 2024
@anorth anorth added this to FilOz May 28, 2024
@rjan90 rjan90 moved this to 📌 Triage in FilOz Jun 4, 2024
@rjan90
Copy link
Contributor

rjan90 commented Jun 4, 2024

This issue is related and might supersede this one: #1548 ?

@rjan90 rjan90 removed the status in FilOz Jun 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cleanup enhancement New feature or request good first issue Good for newcomers P3 testing
Projects
Status: No status
Development

No branches or pull requests

4 participants