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

test(nns): Stop using NnsCanisterUpgrade nns function in integration tests #3841

Merged
merged 2 commits into from
Feb 13, 2025

Conversation

jasonz-dfinity
Copy link
Contributor

@jasonz-dfinity jasonz-dfinity commented Feb 6, 2025

Why

NnsCanisterUpgrade NNS function is deprecated. Stop using it in tests.

What

  • The last param of nns_propose_upgrade_nns_canister is always true. Remove it and simplify the branching.
  • Switch ic_xc_ledger_suite_orchestrator_test to using install_nns_canister_by_proposal and upgrade_nns_canister_with_args_by_proposal

@jasonz-dfinity jasonz-dfinity requested review from a team as code owners February 6, 2025 18:53
Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

If this pull request affects the behavior of any canister owned by
the Governance team, remember to update the corresponding
unreleased_changes.md file(s).

To acknowldge this reminder (and unblock the PR), dismiss this
code review by going to the bottom of the pull request page, and
supply one of the following reasons:

  1. Done.

  2. No canister behavior changes.

@github-actions github-actions bot added the test label Feb 6, 2025
@jasonz-dfinity jasonz-dfinity dismissed github-actions[bot]’s stale review February 6, 2025 18:54

No canister behavior changes

Base automatically changed from jason/NNS1-3491-1 to master February 6, 2025 23:57
@jasonz-dfinity jasonz-dfinity force-pushed the jason/NNS1-3491-2 branch 2 times, most recently from ba8ce35 to b65da0f Compare February 8, 2025 00:21
@ninegua ninegua added the CI_ALL_BAZEL_TARGETS Runs all bazel targets and uploads them to S3 label Feb 12, 2025
Copy link
Member

@ninegua ninegua left a comment

Choose a reason for hiding this comment

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

Since this changed code in rs/tests, I added a tag to trigger all targets.

@jasonz-dfinity jasonz-dfinity added this pull request to the merge queue Feb 13, 2025
Merged via the queue into master with commit e71ace4 Feb 13, 2025
25 checks passed
@jasonz-dfinity jasonz-dfinity deleted the jason/NNS1-3491-2 branch February 13, 2025 16:36
marko-k0 pushed a commit that referenced this pull request Feb 14, 2025
…tests (#3841)

# Why

`NnsCanisterUpgrade` NNS function is deprecated. Stop using it in tests.

# What

* The last param of `nns_propose_upgrade_nns_canister` is always `true`.
Remove it and simplify the branching.
* Switch `ic_xc_ledger_suite_orchestrator_test` to using
`install_nns_canister_by_proposal` and
`upgrade_nns_canister_with_args_by_proposal`

Co-authored-by: Paul Liu <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CI_ALL_BAZEL_TARGETS Runs all bazel targets and uploads them to S3 @cross-chain-team @nns-team test
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants