fix(iota-graphql-e2e-tests): restore graphql e2e tests #1969

kodemartin · 2024-08-21T13:40:08Z

Description of change

This patch does the following:

Fixes a bug introduced by the upgrade of async_graphql in fix(CI): Fix cargo deny #1106 (see cde7061)
Updates the baseline for the e2e tests, which among other things removes the query on the subsidies.
Removes obsolete name service compatibility tests from @iota/graphql-transport

@valeriyr, @miker83z, @pavlo these tests use the iota-transactional-test-runner and so the baseline updates mostly relate to the updated genesis, and different ordering of the results. I would appreciate if one of you could have a quick look to verify that the responses are plausible. To me it seems so.

Links to any relevant issues

Fixes #1580

Type of change

Bug fix (a non-breaking change which fixes an issue)

How the change has been tested

Follow the instructions in the updated README file to run the tests locally.

Change checklist

I have followed the contribution guidelines for this project
I have performed a self-review of my own code
I have commented my code, particularly in hard-to-understand areas
I have made corresponding changes to the documentation
I have checked that new and existing unit tests pass locally with my changes

…tests

…aphql json cursors

… tests

…k_effects tests

valeriyr

Everything seems to be good. I would double-check the suspicions changes just in case.
However, the test changes happened not because of the changes related to this PR.

valeriyr · 2024-08-22T14:59:08Z

crates/iota-graphql-e2e-tests/tests/call/simple.exp

-Contents: iota_system::staking_pool::StakedIota {id: iota::object::UID {id: iota::object::ID {bytes: fake(0,0)}}, pool_id: iota::object::ID {bytes: _}, stake_activation_epoch: 0u64, principal: iota::balance::Balance<iota::iota::IOTA> {value: 1500000000000000u64}}
+Contents: iota_system::validator_cap::UnverifiedValidatorOperationCap {id: iota::object::UID {id: iota::object::ID {bytes: fake(0,0)}}, authorizer_validator_address: validator_0}


When I last worked with these tests I found out that when the iota-framework is changed we need to regenerate these *.exp files because addresses, objects/packages IDs, chain IDs, and objects sequence can be changed. It seems like it happens because the TXs digests are changed in this case.

But in this PR several changes like this need to pay attention. Here we checked the view-object 0,0 that was StakedIota but now it is UnverifiedValidatorOperationCap.

Yes, in this case the source file needs to be manually updated

This is changed in the upstream as well (check d25bb07714) as a result of a protocol change, so I think it is plausible to expect a difference here as well.

view-object 0,y fetches objects created in the init task and I assume UnverifiedValidatorOperationCap is just one of those.

But I didn't go deeper, precisely because the scope of these tests is limited to the verification of the graphql queries from a functional perspective. So we only care that the queries return plausible results, that are the same between repeated runs. And that is why you don't need to update the source file @DaughterOfMars .

In this case, it would be a good solution to add something like this to the related move file:

//# view-object 0,1 <- I am not sure about the index

to check both StakedIota and UnverifiedValidatorOperationCap.

Yes, the changes that affect this test were added earlier in another PR.

I looked through the history, this line was changed several times. It is hard to say what they wanted to check here :)

StakedIota no longer exists. object 0,1 is the upper bound and returns a Coin object.

I think we should just leave it as is and get on in peace 😄

valeriyr · 2024-08-22T15:09:56Z

crates/iota-graphql-e2e-tests/tests/consistency/performance/many_objects.exp

-                "id": "0xfeede6486d923f155071368a2c5d6610d22438eea2e0ad8376f521e1b57a9d88",
-                "value": "89"
+                "id": "0xff98d17d64f8d1d502180c9c4dfea5d08559721afa629883f56aec703563bda1",
+                "value": "372"


It is interesting why the values were changed in this test.

Yes I observed a few reorderings in other results as well. It might be related to how objects are ordered during creation in the transactional-test-runner and hence saved into the database, or to the newest version of async_graphql used.

Important thing is again that this result is now idempotent.

valeriyr · 2024-08-22T15:12:14Z

crates/iota-graphql-e2e-tests/tests/consistency/staked_iota.exp

-gas summary: computation_cost: 1000000, storage_cost: 15412800,  storage_rebate: 1976000, non_refundable_storage_fee: 0
+gas summary: computation_cost: 1000000, storage_cost: 14789600,  storage_rebate: 1976000, non_refundable_storage_fee: 0


The storage cost became lower probably because the system object got smaller after removing the stake subsidy fund. But the rebate is the same for some reason.

Might be something worth investigating as part of the new tokenomics implementation.

Seems like I found an answer. The first rebate for the 0x3::sui_system::SuiSystemState object is 0 and it is updated after the first usage.

docker/pg-services-local/docker-compose.yaml

DaughterOfMars · 2024-08-23T12:13:03Z

crates/iota-graphql-e2e-tests/tests/transactions/system.exp

This one looks like it needs some detailed review

crates/iota-graphql-e2e-tests/tests/objects/pagination.exp

DaughterOfMars · 2024-08-23T12:18:42Z

crates/iota-graphql-e2e-tests/tests/consistency/staked_iota.exp

Another changed test case

DaughterOfMars · 2024-08-23T12:19:27Z

crates/iota-graphql-e2e-tests/tests/consistency/objects_pagination.exp

Another changed test case

kodemartin · 2024-08-23T12:35:18Z

@DaughterOfMars please provide specific suggestions about editing this patch, providing proper justification for any underlying issues that you have identified.

Otherwise, please trust that the set of reviewers already assigned, and the author will do or already did the necessary quality controls.

To quote @valeriyr

the test changes happened not because of the changes related to this PR.

and I will additionally repeat that

because the scope of these tests is limited to the verification of the graphql queries from a functional perspective. So we only care that the queries return plausible results, that are the same between repeated runs.

samuel-rufi

PR looks good to me @kodemartin! Great to have these working e2e tests

DaughterOfMars · 2024-08-23T13:54:10Z

@DaughterOfMars please provide specific suggestions about editing this patch, providing proper justification for any underlying issues that you have identified.

The ones that I mentioned have specific test cases that have now been changed. If you view the associated .move file you will see comments like this:

 # select the 2nd and 3rd objects
# note that order does not correspond
# to order in which objects were created

Which indicate that specific values are expected to be returned. However, the cursors have changed and may have invalidated the tests.

kodemartin · 2024-08-23T14:59:18Z

@DaughterOfMars please provide specific suggestions about editing this patch, providing proper justification for any underlying issues that you have identified.

The ones that I mentioned have specific test cases that have now been changed. If you view the associated .move file you will see comments like this:
 # select the 2nd and 3rd objects
# note that order does not correspond
# to order in which objects were created
Which indicate that specific values are expected to be returned. However, the cursors have changed and may have invalidated the tests.

or the comments might have been rendered obsolete at some point.

Again, the scope of these tests is to test graphql not the network operation. Since we have different system packages, it is expected that the baseline is changed.

If you spot issues with the network operation in the update baseline, please open new issues accordingly so that the language team plans them for resolution.

DaughterOfMars · 2024-08-23T15:03:46Z

@DaughterOfMars please provide specific suggestions about editing this patch, providing proper justification for any underlying issues that you have identified.

The ones that I mentioned have specific test cases that have now been changed. If you view the associated .move file you will see comments like this:
 # select the 2nd and 3rd objects
# note that order does not correspond
# to order in which objects were created
Which indicate that specific values are expected to be returned. However, the cursors have changed and may have invalidated the tests.
or the comments might have been rendered obsolete at some point.

Again, the scope of these tests is to test graphql not the network operation. Since we have different system packages, it is expected that the baseline is changed.

That is not what is happening here. The underlying data has changed and that has invalidated the test cases. The comments are not out of date (except in this PR)

kodemartin · 2024-08-23T15:16:29Z

Could you please add specific comments in the files that will help narrow down what you consider as inconsistencies that should be resolved? Otherwise it is very difficult to move forward with your comments.

Please also provide justification on why these are actually inconsistencies, identifying the associated scope, and help this process by proposing the proper way of treatment.

Would you like me to make the changes for you?

kodemartin · 2024-08-23T15:24:35Z

Could you please add specific comments in the files that will help narrow down what you consider as inconsistencies that should be resolved? Otherwise it is very difficult to move forward with your comments.

Please also provide justification on why these are actually inconsistencies, identifying the associated scope, and help this process by proposing the proper way of treatment.

Would you like me to make the changes for you?

I was pretty specific on what I would like you to do. I do not want you to implement the changes. I want to you to help me understand and narrow down the problems, and provide guidance on possible ways of resolving them.

This is what reviewers are supposed to do: https://iotafoundation.atlassian.net/wiki/spaces/IF/pages/2202337498/Reviewer+s+Guidelines#3.-How-to-write-code-review-comments

miker83z · 2024-08-27T09:56:42Z

Given the results of the PR and its reviews, I think we can safely merge this and open a new task for the Vm&Language team to fix the move files. Ideally, we should introduce some kind of determinism in order to avoid frequent changes.

So, I suggest to close and merge this PR if there are no more suggestions regarding graphql queries from a functional perspective.

CC @valeriyr @kodemartin @DaughterOfMars @samuel-rufi

github-actions · 2024-08-27T14:13:24Z

⚠️ 🦋 Changesets Warning: This PR has changes to public npm packages, but does not contain a changeset. You can create a changeset easily by running pnpm changeset in the root of the IOTA repo, and following the prompts. If your change does not need a changeset (e.g. a documentation-only change), you can ignore this message. This warning will be removed when a changeset is added to this pull request.

Learn more about Changesets.

lzpap · 2024-08-27T14:47:03Z

Given the results of the PR and its reviews, I think we can safely merge this and open a new task for the Vm&Language team to fix the move files. Ideally, we should introduce some kind of determinism in order to avoid frequent changes.

So, I suggest to close and merge this PR if there are no more suggestions regarding graphql queries from a functional perspective.

CC @valeriyr @kodemartin @DaughterOfMars @samuel-rufi

Sounds good. Will you open an issue for the language team then @miker83z ?

* refactor(iota-graphql-e2e-test): update baseline for available_range tests * fix(iota-transactional-test-runner): use no padding while encoding graphql json cursors * refactor(iota-graphql-e2e-test): update baseline for call tests * refactor(iota-graphql-e2e-test): update baseline for consistency tests * refactor(iota-graphql-e2e-test): update baseline for epoch tests * refactor(iota-graphql-e2e-test): update baseline for event_connection tests * refactor(iota-graphql-e2e-test): update baseline for limits tests * refactor(iota-graphql-e2e-test): update baseline for objects tests * refactor(iota-graphql-e2e-test): update baseline for owners tests * refactor(iota-graphql-e2e-test): update baseline for packages tests * refactor(iota-graphql-e2e-test): update baseline for transaction_block_effects tests * refactor(iota-graphql-e2e-test): update baseline for transactions tests * refactor(docker): allow overriding postgres env variables * refactor(iota-graphql-e2e-tests): update README * fixup! refactor(iota-graphql-e2e-tests): update README * fixup! refactor(docker): allow overriding postgres env variables * fix(iota-graphql-e2e-test): fixed `objects/pagination.move` test * fix(graphql-e2e-test): remove obsolete name service tests --------- Co-authored-by: Valerii Reutov <[email protected]> Co-authored-by: Levente Pap <[email protected]>

kodemartin added 14 commits August 21, 2024 16:31

refactor(iota-graphql-e2e-test): update baseline for available_range …

d6332a4

…tests

fix(iota-transactional-test-runner): use no padding while encoding gr…

cde7061

…aphql json cursors

refactor(iota-graphql-e2e-test): update baseline for call tests

4210ae0

refactor(iota-graphql-e2e-test): update baseline for consistency tests

8feebc1

refactor(iota-graphql-e2e-test): update baseline for epoch tests

cf49007

refactor(iota-graphql-e2e-test): update baseline for event_connection…

8844b38

… tests

refactor(iota-graphql-e2e-test): update baseline for limits tests

437fba5

refactor(iota-graphql-e2e-test): update baseline for objects tests

0867350

refactor(iota-graphql-e2e-test): update baseline for owners tests

ae7e976

refactor(iota-graphql-e2e-test): update baseline for packages tests

a89c4f4

refactor(iota-graphql-e2e-test): update baseline for transaction_bloc…

3dd54db

…k_effects tests

refactor(iota-graphql-e2e-test): update baseline for transactions tests

8a42887

refactor(docker): allow overriding postgres env variables

987ad09

refactor(iota-graphql-e2e-tests): update README

a37ce5a

kodemartin requested review from a team as code owners August 21, 2024 13:40

kodemartin requested review from tomxey, sergiupopescu199, samuel-rufi and valeriyr August 21, 2024 13:40

kodemartin added sc-platform Issues related to the Smart Contract Platform group. infrastructure Issues related to the Infrastructure Team labels Aug 21, 2024

kodemartin mentioned this pull request Aug 21, 2024

[Task (CI)]: start postgres server to run graphql e2e tests #1971

Closed

kodemartin added 2 commits August 21, 2024 17:05

fixup! refactor(iota-graphql-e2e-tests): update README

1a6a790

fixup! refactor(docker): allow overriding postgres env variables

5360271

valeriyr approved these changes Aug 22, 2024

View reviewed changes

Merge branch 'develop' into sc-platform/restore-graphql-e2e-tests

2734eaf

thibault-martinez reviewed Aug 23, 2024

View reviewed changes

docker/pg-services-local/docker-compose.yaml Show resolved Hide resolved

DaughterOfMars reviewed Aug 23, 2024

View reviewed changes

crates/iota-graphql-e2e-tests/tests/transactions/system.exp Outdated

Copy link

Contributor

DaughterOfMars Aug 23, 2024

Choose a reason for hiding this comment

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

This one looks like it needs some detailed review

DaughterOfMars reviewed Aug 23, 2024

View reviewed changes

crates/iota-graphql-e2e-tests/tests/objects/pagination.exp Outdated Show resolved Hide resolved

DaughterOfMars reviewed Aug 23, 2024

View reviewed changes

crates/iota-graphql-e2e-tests/tests/consistency/staked_iota.exp

Copy link

Contributor

DaughterOfMars Aug 23, 2024

Choose a reason for hiding this comment

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

Another changed test case

DaughterOfMars reviewed Aug 23, 2024

View reviewed changes

crates/iota-graphql-e2e-tests/tests/consistency/objects_pagination.exp Outdated

Copy link

Contributor

DaughterOfMars Aug 23, 2024

Choose a reason for hiding this comment

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

Another changed test case

samuel-rufi approved these changes Aug 23, 2024

View reviewed changes

fix(iota-graphql-e2e-test): fixed objects/pagination.move test

e4cac8b

samuel-rufi requested a review from thibault-martinez August 26, 2024 14:14

lzpap added 2 commits August 27, 2024 15:30

Merge branch 'develop' into sc-platform/restore-graphql-e2e-tests

70094fe

fix(graphql-e2e-test): remove obsolete name service tests

a9f3499

lzpap requested review from a team as code owners August 27, 2024 14:12

lzpap merged commit e32d93c into develop Aug 27, 2024
37 of 39 checks passed

lzpap deleted the sc-platform/restore-graphql-e2e-tests branch August 27, 2024 14:57

thibault-martinez mentioned this pull request Aug 28, 2024

[Bug (SC-Platform)]: Failing Test - iota-graphql-e2e-tests::tests run_test::consistency/objects_pagination.move #1213

Closed

valeriyr mentioned this pull request Oct 17, 2024

fix(iota-graphql-e2e-tests): restore remaining tests #3371

Merged

6 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(iota-graphql-e2e-tests): restore graphql e2e tests #1969

fix(iota-graphql-e2e-tests): restore graphql e2e tests #1969

kodemartin commented Aug 21, 2024 •

edited by lzpap

Loading

valeriyr left a comment

valeriyr Aug 22, 2024

DaughterOfMars Aug 22, 2024

kodemartin Aug 22, 2024

valeriyr Aug 23, 2024 •

edited

Loading

valeriyr Aug 23, 2024

kodemartin Aug 23, 2024

valeriyr Aug 22, 2024

kodemartin Aug 22, 2024

valeriyr Aug 22, 2024 •

edited

Loading

kodemartin Aug 22, 2024

valeriyr Aug 22, 2024

DaughterOfMars Aug 23, 2024

DaughterOfMars Aug 23, 2024

DaughterOfMars Aug 23, 2024

kodemartin commented Aug 23, 2024

samuel-rufi left a comment

DaughterOfMars commented Aug 23, 2024

kodemartin commented Aug 23, 2024

DaughterOfMars commented Aug 23, 2024

kodemartin commented Aug 23, 2024 •

edited by DaughterOfMars

Loading

kodemartin commented Aug 23, 2024

miker83z commented Aug 27, 2024

github-actions bot commented Aug 27, 2024

lzpap commented Aug 27, 2024 •

edited

Loading

		Contents: iota_system::staking_pool::StakedIota {id: iota::object::UID {id: iota::object::ID {bytes: fake(0,0)}}, pool_id: iota::object::ID {bytes: _}, stake_activation_epoch: 0u64, principal: iota::balance::Balance<iota::iota::IOTA> {value: 1500000000000000u64}}
		Contents: iota_system::validator_cap::UnverifiedValidatorOperationCap {id: iota::object::UID {id: iota::object::ID {bytes: fake(0,0)}}, authorizer_validator_address: validator_0}

		gas summary: computation_cost: 1000000, storage_cost: 15412800, storage_rebate: 1976000, non_refundable_storage_fee: 0
		gas summary: computation_cost: 1000000, storage_cost: 14789600, storage_rebate: 1976000, non_refundable_storage_fee: 0

fix(iota-graphql-e2e-tests): restore graphql e2e tests #1969

fix(iota-graphql-e2e-tests): restore graphql e2e tests #1969

Conversation

kodemartin commented Aug 21, 2024 • edited by lzpap Loading

Description of change

Links to any relevant issues

Type of change

How the change has been tested

Change checklist

valeriyr left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

valeriyr Aug 23, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

valeriyr Aug 22, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kodemartin commented Aug 23, 2024

samuel-rufi left a comment

Choose a reason for hiding this comment

DaughterOfMars commented Aug 23, 2024

kodemartin commented Aug 23, 2024

DaughterOfMars commented Aug 23, 2024

kodemartin commented Aug 23, 2024 • edited by DaughterOfMars Loading

kodemartin commented Aug 23, 2024

miker83z commented Aug 27, 2024

github-actions bot commented Aug 27, 2024

lzpap commented Aug 27, 2024 • edited Loading

kodemartin commented Aug 21, 2024 •

edited by lzpap

Loading

valeriyr Aug 23, 2024 •

edited

Loading

valeriyr Aug 22, 2024 •

edited

Loading

kodemartin commented Aug 23, 2024 •

edited by DaughterOfMars

Loading

lzpap commented Aug 27, 2024 •

edited

Loading