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

bug: Events emitted from within a component cannot be spied on/asserted in tests #5861

Closed
0xNeshi opened this issue Jun 24, 2024 · 6 comments
Labels
bug Something isn't working

Comments

@0xNeshi
Copy link

0xNeshi commented Jun 24, 2024

Bug Report

Cairo version:

scarb 2.6.4+nightly-2024-06-22 (5deb60ac1 2024-06-22)
cairo: 2.6.4 (f3af4cb8d)
sierra: 1.5.0

Current behavior:

Emitting events from components is permitted (see example). But writing tests for these components, and trying to assert that certain events have been emitted is impossible, as the events appear to not be emitted in test environment.

The issue does not occur if the component is converted to be a contract, events to get caught in test environments then.

The issue occurs when using both vanilla test framework and when using Foundry.

Expected behavior:

Events emitted from components can be spied on in tests.

Steps to reproduce:

For vanilla test framework version:

  • switch to scarb 2.6.4+nightly-2024-06-22 (5deb60ac1 2024-06-22)
  • clone https://github.com/misicnenad/component-emit-bug-vanilla
  • open terminal
  • run scarb cairo-test
  • verify that test asserting an event was emitted from a contract is successful
  • verify that test asserting an event was emitted from a component, which has literally the same implementation as the contract version, is failing due to event not being caught

For Foundry test version:

  • switch to scarb 2.6.4+nightly-2024-06-22 (5deb60ac1 2024-06-22)
  • clone https://github.com/misicnenad/component-emit-bug-foundry
  • open terminal
  • run scarb test
  • verify that test asserting an event was emitted from a contract is successful
  • verify that test asserting an event was emitted from a component, which has literally the same implementation as the contract version, is failing due to event not being caught

Other information:

https://github.com/misicnenad/component-emit-bug-foundry
https://github.com/misicnenad/component-emit-bug-vanilla

@0xNeshi
Copy link
Author

0xNeshi commented Jun 24, 2024

Was just playing around and I found out that adding #[flat] above the component's event inside the contract that imports it fixes the issue**

#[starknet::component]
pub mod counter_component {
    #[event]
    #[derive(Drop, Debug, PartialEq, starknet::Event)]
    pub enum Event {
        CounterIncreased: CounterIncreased
    }

    #[derive(Drop, Debug, PartialEq, starknet::Event)]
    pub struct CounterIncreased {
        pub new_value: u32,
    }

    // ...
}

#[starknet::contract]
pub mod MockContract {
    // ...
    #[event]
    #[derive(Drop, starknet::Event)]
    pub enum Event {
        #[flat]
        CounterEvent: counter_component::Event
    }
    // ...
}

The tests now pass and correctly spy on events. It seems kind of counter-intuitive imo.

Is this expected behavior?

@orizi
Copy link
Collaborator

orizi commented Jun 24, 2024

There is no bug - or need for the flat.
the issue is you are trying to read the wrong event from the data.
you need to pop an event of the type of the contract (as that is the event that is actually emitted).
you are instead popping some inner event.
in your case MockContract::Event should be used instead of Counter::Event.

@orizi orizi closed this as completed Jun 24, 2024
@0xNeshi
Copy link
Author

0xNeshi commented Jun 24, 2024

@orizi I don't think that's it. If that were the case, I would receive an error message like: "Expected event of type `MockContract::Event`, but found `Counter::Event`".

Instead, I'm getting the following error message:

scarb cairo-test -f test_increment_event_component
   Compiling test(comp_event_unittest) comp_event v0.1.0 (/home/nenad/repos/cairo_projects/comp_internals/Scarb.toml)
    Finished release target(s) in 2 seconds
testing comp_event ...
running 1 test
test comp_event::tests::test_increment_event_component ... fail (gas usage est.: 1093000)
failures:
   comp_event::tests::test_increment_event_component - Panicked with "assertion `starknet::testing::pop_log(counter.contract_address) == Option::Some(
                counter_component::Event::CounterIncreased(
                    counter_component::CounterIncreased { new_value: 1 }
                )
            )` failed.
starknet::testing::pop_log(counter.contract_address): Option::None(())
Option::Some(
                counter_component::Event::CounterIncreased(
                    counter_component::CounterIncreased { new_value: 1 }
                )
            ): Option::Some(Event::CounterIncreased(CounterIncreased { new_value: 1 }))".

Error: test result: FAILED. 0 passed; 1 failed; 0 ignored

Notice the line starknet::testing::pop_log(counter.contract_address): Option::None(()) -> this implies that no event was emitted. Or am I not understanding this correctly?

Can you please post what you regard as the fixed version of the event configuration in tests?

@orizi
Copy link
Collaborator

orizi commented Jun 24, 2024

this indeed means an event has not been emitted.
the original thing described is still not a bug - feel free to ask a question on discord about the actual issue you are having.

your current pop + comparison seems correct type-wise.
(as you are comparing to the event type, the event type would be popped).

@0xNeshi
Copy link
Author

0xNeshi commented Jun 24, 2024

Will post a question on Discord regarding this, I may be doing something wrong.

@0xNeshi
Copy link
Author

0xNeshi commented Jun 25, 2024

@orizi I got an answer on Discord that resolved my issue (see https://discord.com/channels/793094838509764618/793094838987128844/1255043119355924500).

The problem was basically as you described it, I just had to update the event signature to use MockContract::Event.

I guess the problem is just that confusing error message; from my previous comment:

Notice the line starknet::testing::pop_log(counter.contract_address): Option::None(()) -> this implies that no event was emitted.

Because of this message, I naturally assumed that... no event was emitted. So I didn't even bother tinkering with the event signatures.

The error message should have stated that event signatures did not match, that would immediately point me to investigate that part of the implementation.
Should I open an issue about this error message? This update would make the error much clearer.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants