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

Fix assert_expected_events macro #7913

Draft
wants to merge 10 commits into
base: master
Choose a base branch
from

Conversation

claravanstaden
Copy link
Contributor

@claravanstaden claravanstaden commented Mar 13, 2025

Description

The assert_expected_events does not correctly check event fields. Even though the provided field values do not match, the macro did not raise an error.

Review Notes

The meet_conditions variable kept being reset to true, which means that conditions not met would be overwritten with a positive value.

Closes: #2460

@bkontur
Copy link
Contributor

bkontur commented Mar 13, 2025

@claravanstaden can you also please check this #2460 - if it is related or the same problem? And if so, please close with this PR?

@claravanstaden
Copy link
Contributor Author

Closing as a duplicate of #2460.

@acatangiu
Copy link
Contributor

#2460 is just an issue, not a PR, this PR is not a duplicate

@acatangiu acatangiu reopened this Mar 13, 2025
@claravanstaden
Copy link
Contributor Author

I think I fixed the macro, but need to fix all the failing tests now. 😅

@acatangiu
Copy link
Contributor

I think I fixed the macro, but need to fix all the failing tests now. 😅

🙈 that is some noble task ❤️

@acatangiu
Copy link
Contributor

I think I fixed the macro, but need to fix all the failing tests now. 😅

if any of them are complicated to fix, feel free to tag the git blame test/check author on them for help

@claravanstaden
Copy link
Contributor Author

cargo test -p asset-hub-rococo-integration-tests -p asset-hub-westend-integration-tests -p bridge-hub-rococo-integration-tests -p bridge-hub-westend-integration-tests -p collectives-westend-integration-tests -p coretime-rococo-integration-tests -p coretime-westend-integration-tests -p people-westend-integration-tests -p people-rococo-integration-tests

failures:
    tests::hybrid_transfers::bidirectional_teleport_foreign_asset_between_para_and_asset_hub_using_explicit_transfer_types
    tests::hybrid_transfers::transfer_foreign_assets_from_para_to_asset_hub
    tests::hybrid_transfers::transfer_native_asset_from_relay_to_para_through_asset_hub
    tests::reserve_transfer::reserve_transfer_multiple_assets_from_asset_hub_to_para
    tests::reserve_transfer::reserve_transfer_multiple_assets_from_para_to_asset_hub
    tests::reserve_transfer::reserve_transfer_native_asset_from_para_to_asset_hub
    tests::reserve_transfer::reserve_transfer_native_asset_from_para_to_relay
    tests::reserve_transfer::reserve_transfer_native_asset_from_relay_to_para
    tests::reserve_transfer::reserve_transfer_usdt_from_asset_hub_to_para
    tests::reserve_transfer::reserve_transfer_usdt_from_para_to_para_through_asset_hub
    tests::teleport::bidirectional_teleport_foreign_assets_between_para_and_asset_hub
    tests::teleport::limited_teleport_native_assets_from_system_para_to_relay_fails
    tests::xcm_fee_estimation::multi_hop_pay_fees_works

assert_expected_events!(
AssetHubRococo,
vec![
RuntimeEvent::Balances(
pallet_balances::Event::Burned { amount, .. }
) => {
amount: *amount == test.args.amount,
amount: *amount > test.args.amount * 90/100,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@franciscoaguirre for multi_hop_pay_fees_works, the expected amount for the burn event is 1000000000000, but the actual amount in the event is 966873920000. Probably a part of it was used to pay for fees? I want to update the tests but just make sure with you if it is OK. I want to remove the amount or change it to something like: amount: *amount >= test.args.amount *.9 (i.e. the amount burned should at least be higher than 90% of the test amount).

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.

assert_expected_events! gives false positives on inner enum values
3 participants