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.

Sign up for GitHub

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

Jump to bottom

refactor(nervous-system): Use Sns type in integration tests rather than DeployedSns #1589

Merged
merged 1 commit into from
Sep 25, 2024
Merged

refactor(nervous-system): Use Sns type in integration tests rather than DeployedSns #1589

merged 1 commit into from
Sep 25, 2024

Conversation

anchpop
Copy link
Contributor

@anchpop anchpop commented Sep 19, 2024 •
edited
Loading

Unlike DeployedSns, the Sns type has no optional fields. This means we can remove the repetitive unwrapping in every test that deploys an SNS. The Sns type also has some utility functions on it that can be used in the integration tests once #1590 is merged

@anchpop anchpop marked this pull request as ready for review September 19, 2024 22:38
@anchpop anchpop requested a review from a team as a code owner September 19, 2024 22:38
@anchpop anchpop mentioned this pull request Sep 19, 2024
Merged
@anchpop
Copy link
Contributor Author

anchpop commented Sep 19, 2024 •
edited
Loading

Note to reviewer: CI is failing, but the cause is something that should not be fixed as part of this PR. (Instead I will fix it in a separate PR.) It is failing because ic-sns-wasm has a test feature, and pocketic uses it with the test feature enabled and ic-nervous-system-agent doesn't, leading to two different versions of ic-sns-wasm in the dependency stack (since bazel doesn't support merging features)

@max-dfinity
Copy link
Contributor

Note to reviewer: CI is failing, but the cause is something that should not be fixed as part of this PR. (Instead I will fix it in a separate PR.) It is failing because ic-sns-wasm has a test feature, and pocketic uses it with the test feature enabled and ic-nervous-system-agent doesn't, leading to two different versions of ic-sns-wasm in the dependency stack (since bazel doesn't support merging features)

We should remove all the test_features crates - that will dramatically simplify our lives. It was a mistake introducing them in the first place, but we didn't have a faster option at the time.

github-merge-queue bot pushed a commit that referenced this pull request Sep 20, 2024
@anchpop
refactor(nervous-system-agent): Add pocket-ic support to ic-nervous-s…
682f021 
…ystem-agent (#1590)

This will allow us to use ic-nervous-system-agent helper functions from
within tests built on pocket-ic. This PR synergizes with #1589, which
starts to use ic-nervous-system-agent types from within
ic-nervous-system-integration-tests
frankdavid pushed a commit that referenced this pull request Sep 25, 2024
@anchpop @frankdavid
refactor(nervous-system-agent): Add pocket-ic support to ic-nervous-s…
34cd6ad 
…ystem-agent (#1590)

This will allow us to use ic-nervous-system-agent helper functions from
within tests built on pocket-ic. This PR synergizes with #1589, which
starts to use ic-nervous-system-agent types from within
ic-nervous-system-integration-tests
@anchpop anchpop force-pushed the @anchpop/use-sns-type branch 4 times, most recently from cf66b83 to 4a005f8 Compare September 25, 2024 19:34
@anchpop
Copy link
Contributor Author

anchpop commented Sep 25, 2024

Note to reviewer: CI is failing, but the cause is something that should not be fixed as part of this PR. (Instead I will fix it in a separate PR.) It is failing because ic-sns-wasm has a test feature, and pocketic uses it with the test feature enabled and ic-nervous-system-agent doesn't, leading to two different versions of ic-sns-wasm in the dependency stack (since bazel doesn't support merging features)

We should remove all the test_features crates - that will dramatically simplify our lives. It was a mistake introducing them in the first place, but we didn't have a faster option at the time.

Enough of the test_features crates have been removed that this PR is no longer blocked!

@anchpop anchpop added this pull request to the merge queue Sep 25, 2024
Merged via the queue into master with commit 35153c7 Sep 25, 2024
24 checks passed
@anchpop anchpop deleted the @anchpop/use-sns-type branch September 25, 2024 20:42
levifeldman pushed a commit to levifeldman/ic that referenced this pull request Oct 1, 2024
@anchpop @levifeldman
refactor(nervous-system-agent): Add pocket-ic support to ic-nervous-s…
0663326 
…ystem-agent (dfinity#1590)

This will allow us to use ic-nervous-system-agent helper functions from
within tests built on pocket-ic. This PR synergizes with dfinity#1589, which
starts to use ic-nervous-system-agent types from within
ic-nervous-system-integration-tests
levifeldman pushed a commit to levifeldman/ic that referenced this pull request Oct 1, 2024
@anchpop @levifeldman
refactor(nervous-system): Use Sns type in integration tests rather …
582daa7 
…than `DeployedSns` (dfinity#1589)

Unlike `DeployedSns`, the `Sns` type has no optional fields. This means
we can remove the repetitive unwrapping in every test that deploys an
SNS. The `Sns` type also has some utility functions on it that can be
used in the integration tests once dfinity#1590 is merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@aterga aterga aterga approved these changes

Assignees
No one assigned
Labels
@nns-team refactor
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@anchpop @max-dfinity @aterga