Issues/1566 simplify event testing

[mootz12]

What

Updates testutils::Events read functions to use the xdr::ContractEvent type and deprecates the current all function. Adds helpers to contractevent structs to easily transform event structs into xdr::ContractEvent's for easy comparison during tests.

Finally, this also adds function to testutils::Events to allow test writers to easily pull events published by a specific contract_id.

As a side effect, using xdr also allows the assertion debug info to display what is different between events.

For example:

left: [ContractEvent { ext: V0, contract_id: Some(ContractId(Hash(0000000000000000000000000000000000000000000000000000000000000001))), type_: Contract, body: V0(ContractEventV0 { topics: VecM([Symbol(ScSymbol(StringM(transfer))), Address(Contract(ContractId(Hash(0000000000000000000000000000000000000000000000000000000000000002)))), Address(Account(AccountId(PublicKeyTypeEd25519(Uint256(0000000000000000000000000000000000000000000000000000000000000003)))))]), data: Map(Some(ScMap(VecM([ScMapEntry { key: Symbol(ScSymbol(StringM(amount))), val: I128(Int128Parts { hi: 0, lo: 1 }) }, ScMapEntry { key: Symbol(ScSymbol(StringM(to_muxed_id))), val: U64(1) }])))) }) }]
 right: [ContractEvent { ext: V0, contract_id: Some(ContractId(Hash(0000000000000000000000000000000000000000000000000000000000000001))), type_: Contract, body: V0(ContractEventV0 { topics: VecM([Symbol(ScSymbol(StringM(transfer))), Address(Contract(ContractId(Hash(0000000000000000000000000000000000000000000000000000000000000002)))), Address(Account(AccountId(PublicKeyTypeEd25519(Uint256(0000000000000000000000000000000000000000000000000000000000000003)))))]), data: Map(Some(ScMap(VecM([ScMapEntry { key: Symbol(ScSymbol(StringM(amount))), val: I128(Int128Parts { hi: 0, lo: 0 }) }, ScMapEntry { key: Symbol(ScSymbol(StringM(to_muxed_id))), val: U64(1) }])))) }) }]

instead of

left: Vec(Ok((Contract(CAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAD2KM), Vec(Ok(Symbol(transfer)), Ok(Address(obj#35)), Ok(Address(obj#37))), Map(obj#43))))
right: Vec(Ok((Contract(CAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAD2KM), Vec(Ok(Symbol(transfer)), Ok(Address(obj#11)), Ok(Address(obj#29))), Map(obj#53))))

Why

Helps test write easier event comparison.

For a lighter solution, we can also consider #1634 . This just adds a few utils to make building events in tests easier.

Known limitations

None

[tupui]

Great to see some work on that!!

I personally would prefer your alternate option 1. Would there be a way to just not care about the contract ID? I would suspect that in lot of cases people would not care much about asserting that no? In basic instances, I actually might just want to check that I have an instance of a specific event and don't need to check the inner values.

[mootz12]

Would there be a way to just not care about the contract ID?

You would need to compare the parts of the event you care about manually within the test.

e.g.

        let event = Transfer {
            from: from.clone(),
            to: to.address(),
            amount,
            to_muxed_id: to.id(),
        };
        let (_, topics, data) = env.events().all()[0]
        assert_eq!(topics, event.topics());
        assert_eq!(data, event.data());

I don't think it would be a good idea for contract IDs to be omitted from the env.events() in general.

In basic instances, I actually might just want to check that I have an instance of a specific event and don't need to check the inner values.

You would also likely just compare the first topic, or perform some any call into the events to find one with your expected topic.

One advantage of the ContractEvent struct, is that you can do things like:

        assert!(env.events().all().contains(
            &Transfer {
                from: from.clone(),
                to: to.clone(),
                amount,
                to_muxed_id: None,
            }
            .to_contract_event(&env, &contract_id)
        ));

This is neat for composed contracts where their event might be one of many in the test transaction.

i didn't try super hard to see if we could get this to work for option 2, where to_contract_event just builds the existing tuple that events().all() returns. Currently, it has issues due to trying to compare Val. Maybe that is the better option.

[tupui]

Would there be a way to just not care about the contract ID?

You would need to compare the parts of the event you care about manually within the test.
...

I don't think it would be a good idea for contract IDs to be omitted from the env.events() in general.

Fair enough, your example is a simple enough syntax 👍

One advantage of the ContractEvent struct, is that you can do things like:

        assert!(env.events().all().contains(
            &Transfer {
                from: from.clone(),
                to: to.clone(),
                amount,
                to_muxed_id: None,
            }
            .to_contract_event(&env, &contract_id)
        ));

This is neat for composed contracts where their event might be one of many in the test transaction.

This would be great to be able to do some contains like that 💯

One question, is there actually an advantage in having env.events().all() not return directly a vec of struct? Or maybe we could have a env.events_test().all() which would do the handy manipulation for us.

[mootz12]

@tupui - thought of an alternative solution this morning, curious if you have any thoughts here as well:

#1634

[tupui]

assert!(event.matches(&env, &id, &published));

This is nice and simpler, I like it!

I am not sure about the wording matches, as at first I was thinking about a Rust match. It's more an inclusion so contains above was more obvious on what was going on. Maybe event.published, event.published_in or something like that would be clearer and directly link to event.publish?

[leighmcculloch]

I haven't reviewed the whole diff yet, but some thoughts inline.

[mootz12]

closing in favor of #1638