Skip to content

Commit

Permalink
refactor(nervous-system): Remove ic-sns-governance and ic-sns-init's …
Browse files Browse the repository at this point in the history
…test_feature configurations (#1606)

[← Previous PR](#1609)

There are two reasons to have a --test_feature target:

1. The crate itself has different functionality when cfg(feature =
"test") is true
2. The crate is a library that depends on crate that has a
--test_feature target, and needs to give the choice to its consumer
whether the dependency has --test_feature enabled.

The second rule is "viral" which has lead to a massive outbreak of
crates with a `--test_feature` target. In fact most of our crates that
had a --test_feature target were in the second category.

The root cause of most of these was that SNS Governance's library had a
`--test_feature` target. This PR:

1. Moves the logic for determining whether we are in "test mode" to the
canister crate (from the library crate)
2. Remove's SNS Governance's library's --test-feature target
3. Remove's SNS Init's --test-feature target, which did nothing after
#1609
4. Reverses the viral outbreak of --test-feature now that these two
crates no longer needed it. The following crates have had their
--test-feature removed entirely:
   1. `//rs/sns/init:init--test_feature`
   2. `//rs/sns/governance:governance--test_feature`
   4. `//rs/nns/governance/api:api--test_feature`
   5. `//rs/nns/sns-wasm:sns-wasm--test_feature`
   6.  `//rs/sns/cli:cli--test_feature`
   7.  `//rs/sns/swap:swap--test_feature`
   8.  `//rs/nns/governance/init:init--test_feature`
   8.  `//rs/nns/gtc:gtc--test_feature`

And some others no longer depend on any --test_feature crates, which is
useful for development
  • Loading branch information
anchpop authored Sep 25, 2024
1 parent 5610c60 commit 09e7929
Show file tree
Hide file tree
Showing 19 changed files with 104 additions and 294 deletions.
1 change: 0 additions & 1 deletion publish/binaries/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,6 @@ ALL_BINARIES = {
"sandbox_launcher": "//rs/canister_sandbox:sandbox_launcher",
"sns": "//rs/sns/cli:sns",
"sns-audit": "//rs/sns/audit:sns-audit",
"sns-test-feature": "//rs/sns/cli:sns--test_feature",
"state-tool": "//rs/state_tool:state-tool",
"systemd-journal-gatewayd-shim": "//rs/boundary_node/systemd_journal_gatewayd_shim:systemd-journal-gatewayd-shim",
"drun": "//rs/drun:drun",
Expand Down
28 changes: 5 additions & 23 deletions rs/nervous_system/integration_tests/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -11,12 +11,17 @@ BASE_DEPENDENCIES = [
"//rs/nervous_system/proto",
"//rs/nervous_system/root",
"//rs/nns/common",
"//rs/nns/governance/api",
"//rs/nns/sns-wasm",
"//rs/rosetta-api/icp_ledger",
"//rs/rosetta-api/icrc1",
"//rs/rosetta-api/icrc1/index-ng",
"//rs/rosetta-api/icrc1/tokens_u64",
"//rs/rosetta-api/ledger_core",
"//rs/sns/governance",
"//rs/sns/init",
"//rs/sns/root",
"//rs/sns/swap",
"//rs/test_utilities/load_wasm",
"//rs/types/base_types",
"//rs/types/management_canister_types",
Expand Down Expand Up @@ -48,32 +53,9 @@ BASE_DEPENDENCIES = [
],
})

# Each target declared in this file may choose either these (release-ready)
# dependencies (`DEPENDENCIES`), or `DEPENDENCIES_WITH_TEST_FEATURES` feature previews.

# Currently unused.
# DEPENDENCIES = BASE_DEPENDENCIES + [
# "//rs/sns/init",
# "//rs/nns/sns-wasm",
# "//rs/nns/handlers/root/impl:root",
# "//rs/sns/swap",
# ] + select({
# "@rules_rust//rust/platform:wasm32-unknown-unknown": [],
# "//conditions:default": [
# "//rs/nns/constants",
# "//rs/nns/test_utils",
# "//rs/nns/gtc",
# ],
# })

DEPENDENCIES_WITH_TEST_FEATURES = BASE_DEPENDENCIES + [
"//rs/sns/init:init--test_feature",
"//rs/nns/governance:governance--test_feature",
"//rs/nns/governance/api:api--test_feature",
"//rs/nns/sns-wasm:sns-wasm--test_feature",
"//rs/nns/handlers/root/impl:root--test_feature",
"//rs/sns/governance:governance--test_feature",
"//rs/sns/swap:swap--test_feature",
] + select({
"@rules_rust//rust/platform:wasm32-unknown-unknown": [],
"//conditions:default": [
Expand Down
35 changes: 11 additions & 24 deletions rs/nns/governance/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ filegroup(
)

# See rs/nervous_system/feature_test.md
BASE_DEPENDENCIES = [
DEPENDENCIES = [
# Keep sorted.
"//rs/crypto/getrandom_for_wasm",
"//rs/crypto/sha2",
Expand All @@ -30,16 +30,21 @@ BASE_DEPENDENCIES = [
"//rs/nns/cmc",
"//rs/nns/common",
"//rs/nns/constants",
"//rs/nns/governance/api",
"//rs/nns/governance/init",
"//rs/nns/gtc_accounts",
"//rs/nns/handlers/root/interface",
"//rs/nns/sns-wasm",
"//rs/protobuf",
"//rs/registry/canister",
"//rs/rosetta-api/icp_ledger",
"//rs/rosetta-api/ledger_core",
"//rs/rust_canisters/dfn_http_metrics",
"//rs/rust_canisters/http_types",
"//rs/rust_canisters/on_wire",
"//rs/sns/init",
"//rs/sns/root",
"//rs/sns/swap",
"//rs/types/base_types",
"//rs/types/management_canister_types",
"//rs/types/types",
Expand Down Expand Up @@ -76,24 +81,6 @@ BASE_DEPENDENCIES = [
],
})

# Each target declared in this file may choose either these (release-ready)
# dependencies (`DEPENDENCIES`), or `DEPENDENCIES_WITH_TEST_FEATURES` feature previews.
DEPENDENCIES = BASE_DEPENDENCIES + [
"//rs/nns/governance/api",
"//rs/nns/sns-wasm",
"//rs/sns/init",
"//rs/nns/governance/init",
"//rs/sns/swap",
]

DEPENDENCIES_WITH_TEST_FEATURES = BASE_DEPENDENCIES + [
"//rs/nns/governance/api:api--test_feature",
"//rs/nns/sns-wasm:sns-wasm--test_feature",
"//rs/sns/init:init--test_feature",
"//rs/sns/swap:swap--test_feature",
"//rs/nns/governance/init:init--test_feature",
]

MACRO_DEPENDENCIES = [
# Keep sorted.
"//rs/nervous_system/common/build_metadata",
Expand Down Expand Up @@ -169,7 +156,7 @@ rust_library(
crate_name = "ic_nns_governance",
proc_macro_deps = MACRO_DEPENDENCIES,
version = "0.9.0",
deps = DEPENDENCIES_WITH_TEST_FEATURES + [
deps = DEPENDENCIES + [
":build_script",
],
)
Expand Down Expand Up @@ -227,7 +214,7 @@ rust_canister(
crate_features = ["test"],
proc_macro_deps = MACRO_DEPENDENCIES,
service_file = ":canister/governance_test.did",
deps = DEPENDENCIES_WITH_TEST_FEATURES + [
deps = DEPENDENCIES + [
":build_script",
":governance--test_feature",
"//rs/nervous_system/canisters",
Expand Down Expand Up @@ -275,7 +262,7 @@ rust_test(
crate_features = ["test"],
crate_root = "canister/canister.rs",
proc_macro_deps = MACRO_DEPENDENCIES + MACRO_DEV_DEPENDENCIES,
deps = DEPENDENCIES_WITH_TEST_FEATURES + DEV_DEPENDENCIES + [
deps = DEPENDENCIES + DEV_DEPENDENCIES + [
":build_script",
":governance--test_feature",
"//rs/nervous_system/canisters",
Expand All @@ -298,7 +285,7 @@ rust_test(
aliases = ALIASES,
crate_features = ["test"],
proc_macro_deps = MACRO_DEPENDENCIES + MACRO_DEV_DEPENDENCIES,
deps = DEPENDENCIES_WITH_TEST_FEATURES + DEV_DEPENDENCIES + [
deps = DEPENDENCIES + DEV_DEPENDENCIES + [
":build_script",
],
)
Expand Down Expand Up @@ -341,7 +328,7 @@ rust_test_suite_with_extra_srcs(
"tests/*/*.rs",
]) + ["tests/fake.rs"],
proc_macro_deps = MACRO_DEPENDENCIES + MACRO_DEV_DEPENDENCIES,
deps = [":governance--test_feature"] + DEPENDENCIES_WITH_TEST_FEATURES + DEV_DEPENDENCIES + [
deps = [":governance--test_feature"] + DEPENDENCIES + DEV_DEPENDENCIES + [
":build_script",
],
)
15 changes: 3 additions & 12 deletions rs/nns/governance/api/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ load("@rules_rust//rust:defs.bzl", "rust_library")
package(default_visibility = ["//visibility:public"])

# See rs/nervous_system/feature_test.md
BASE_DEPENDENCIES = [
DEPENDENCIES = [
# Keep sorted.
"//rs/crypto/sha2",
"//rs/nervous_system/clients",
Expand All @@ -13,6 +13,7 @@ BASE_DEPENDENCIES = [
"//rs/protobuf",
"//rs/rosetta-api/icp_ledger",
"//rs/sns/root",
"//rs/sns/swap:swap",
"//rs/types/base_types",
"//rs/types/types",
"//rs/utils",
Expand All @@ -26,16 +27,6 @@ BASE_DEPENDENCIES = [
"@crate_index//:strum",
]

# Each target declared in this file may choose either these (release-ready)
# dependencies (`DEPENDENCIES`), or `DEPENDENCIES_WITH_TEST_FEATURES` feature previews.
DEPENDENCIES = BASE_DEPENDENCIES + [
"//rs/sns/swap",
]

DEPENDENCIES_WITH_TEST_FEATURES = BASE_DEPENDENCIES + [
"//rs/sns/swap:swap--test_feature",
]

MACRO_DEPENDENCIES = [
# Keep sorted.
"@crate_index//:strum_macros",
Expand Down Expand Up @@ -67,5 +58,5 @@ rust_library(
crate_name = "ic_nns_governance_api",
proc_macro_deps = MACRO_DEPENDENCIES,
version = "0.9.0",
deps = DEPENDENCIES_WITH_TEST_FEATURES,
deps = DEPENDENCIES,
)
15 changes: 3 additions & 12 deletions rs/nns/governance/init/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -3,28 +3,19 @@ load("@rules_rust//rust:defs.bzl", "rust_library")
package(default_visibility = ["//visibility:public"])

# See rs/nervous_system/feature_test.md
BASE_DEPENDENCIES = [
DEPENDENCIES = [
# Keep sorted.
"//rs/nervous_system/common",
"//rs/nervous_system/common/test_keys",
"//rs/nns/common",
"//rs/nns/governance/api",
"//rs/rosetta-api/icp_ledger",
"//rs/types/base_types",
"@crate_index//:csv",
"@crate_index//:rand",
"@crate_index//:rand_chacha",
]

# Each target declared in this file may choose either these (release-ready)
# dependencies (`DEPENDENCIES`), or `DEPENDENCIES_WITH_TEST_FEATURES` feature previews.
DEPENDENCIES = BASE_DEPENDENCIES + [
"//rs/nns/governance/api",
]

DEPENDENCIES_WITH_TEST_FEATURES = BASE_DEPENDENCIES + [
"//rs/nns/governance/api:api--test_feature",
]

MACRO_DEPENDENCIES = [
# Keep sorted.
]
Expand Down Expand Up @@ -54,5 +45,5 @@ rust_library(
crate_name = "ic_nns_governance_init",
proc_macro_deps = MACRO_DEPENDENCIES,
version = "0.9.0",
deps = DEPENDENCIES_WITH_TEST_FEATURES,
deps = DEPENDENCIES,
)
15 changes: 3 additions & 12 deletions rs/nns/gtc/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -12,14 +12,15 @@ filegroup(
)

# See rs/nervous_system/feature_test.md
BASE_DEPENDENCIES = [
DEPENDENCIES = [
# Keep sorted.
"//rs/crypto/getrandom_for_wasm",
"//rs/crypto/secp256k1",
"//rs/crypto/sha2",
"//rs/nervous_system/common",
"//rs/nns/common",
"//rs/nns/constants",
"//rs/nns/governance/api",
"//rs/nns/gtc_accounts",
"//rs/rosetta-api/icp_ledger",
"//rs/rust_canisters/dfn_candid",
Expand All @@ -36,16 +37,6 @@ BASE_DEPENDENCIES = [
"@crate_index//:sha3",
]

# Each target declared in this file may choose either these (release-ready)
# dependencies (`DEPENDENCIES`), or `DEPENDENCIES_WITH_TEST_FEATURES` feature previews.
DEPENDENCIES = BASE_DEPENDENCIES + [
"//rs/nns/governance/api",
]

DEPENDENCIES_WITH_TEST_FEATURES = BASE_DEPENDENCIES + [
"//rs/nns/governance/api:api--test_feature",
]

MACRO_DEPENDENCIES = [
# Keep sorted.
"//rs/nervous_system/common/build_metadata",
Expand Down Expand Up @@ -113,7 +104,7 @@ rust_library(
crate_name = "ic_nns_gtc",
proc_macro_deps = MACRO_DEPENDENCIES,
version = "0.9.0",
deps = DEPENDENCIES_WITH_TEST_FEATURES + [":build_script"],
deps = DEPENDENCIES + [":build_script"],
)

rust_canister(
Expand Down
13 changes: 4 additions & 9 deletions rs/nns/integration_tests/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -12,13 +12,17 @@ BASE_DEPENDENCIES = [
"//rs/nervous_system/common/test_keys",
"//rs/nns/cmc",
"//rs/nns/common",
"//rs/nns/governance/api",
"//rs/nns/governance/init",
"//rs/nns/handlers/lifeline/impl:lifeline",
"//rs/nns/sns-wasm",
"//rs/rosetta-api/icp_ledger",
"//rs/rosetta-api/ledger_core",
"//rs/rust_canisters/dfn_candid",
"//rs/rust_canisters/dfn_core",
"//rs/rust_canisters/dfn_json",
"//rs/rust_canisters/dfn_protobuf",
"//rs/sns/swap",
"//rs/types/base_types",
"@crate_index//:assert_matches",
"@crate_index//:bytes",
Expand Down Expand Up @@ -76,12 +80,7 @@ BASE_DEPENDENCIES = [
DEPENDENCIES = BASE_DEPENDENCIES + [
# Keep sorted.
"//rs/nns/governance",
"//rs/nns/governance/api",
"//rs/nns/governance/init",
"//rs/nns/handlers/root/impl:root",
"//rs/nns/sns-wasm",
"//rs/sns/init",
"//rs/sns/swap",
] + select({
"@rules_rust//rust/platform:wasm32-unknown-unknown": [],
"//conditions:default": [
Expand All @@ -93,12 +92,8 @@ DEPENDENCIES = BASE_DEPENDENCIES + [
DEPENDENCIES_WITH_TEST_FEATURES = BASE_DEPENDENCIES + [
# Keep sorted.
"//rs/nns/governance:governance--test_feature",
"//rs/nns/governance/api:api--test_feature",
"//rs/nns/governance/init:init--test_feature",
"//rs/nns/handlers/root/impl:root--test_feature",
"//rs/nns/sns-wasm:sns-wasm--test_feature",
"//rs/sns/init:init--test_feature",
"//rs/sns/swap:swap--test_feature",
] + select({
"@rules_rust//rust/platform:wasm32-unknown-unknown": [],
"//conditions:default": [
Expand Down
39 changes: 3 additions & 36 deletions rs/nns/sns-wasm/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ filegroup(
)

# See rs/nervous_system/feature_test.md
BASE_DEPENDENCIES = [
DEPENDENCIES = [
# Keep sorted.
"//packages/icrc-ledger-types:icrc_ledger_types",
"//rs/crypto/sha2",
Expand All @@ -24,6 +24,8 @@ BASE_DEPENDENCIES = [
"//rs/rust_canisters/dfn_candid",
"//rs/rust_canisters/dfn_core",
"//rs/rust_canisters/dfn_http_metrics",
"//rs/sns/governance",
"//rs/sns/init",
"//rs/sns/root",
"//rs/types/base_types",
"//rs/types/management_canister_types",
Expand All @@ -42,18 +44,6 @@ BASE_DEPENDENCIES = [
"@crate_index//:serde_json",
]

# Each target declared in this file may choose either these (release-ready)
# dependencies (`DEPENDENCIES`), or `DEPENDENCIES_WITH_TEST_FEATURES` feature previews.
DEPENDENCIES = BASE_DEPENDENCIES + [
"//rs/sns/governance",
"//rs/sns/init",
]

DEPENDENCIES_WITH_TEST_FEATURES = BASE_DEPENDENCIES + [
"//rs/sns/governance:governance--test_feature",
"//rs/sns/init:init--test_feature",
]

MACRO_DEPENDENCIES = [
# Keep sorted.
"@crate_index//:async-trait",
Expand Down Expand Up @@ -104,20 +94,6 @@ rust_library(
deps = DEPENDENCIES,
)

rust_library(
name = "sns-wasm--test_feature",
srcs = glob([
"src/**",
"gen/**",
]),
aliases = ALIASES,
crate_features = ["test"],
crate_name = "ic_sns_wasm",
proc_macro_deps = MACRO_DEPENDENCIES,
version = "1.0.0",
deps = DEPENDENCIES_WITH_TEST_FEATURES,
)

rust_canister(
name = "sns-wasm-canister",
srcs = ["canister/canister.rs"],
Expand Down Expand Up @@ -145,15 +121,6 @@ rust_test(
deps = DEPENDENCIES + DEV_DEPENDENCIES,
)

rust_test(
name = "sns-wasm_test--test_feature",
aliases = ALIASES,
crate = ":sns-wasm--test_feature",
crate_features = ["test"],
proc_macro_deps = MACRO_DEPENDENCIES + MACRO_DEV_DEPENDENCIES,
deps = DEPENDENCIES_WITH_TEST_FEATURES + DEV_DEPENDENCIES,
)

rust_ic_test_suite_with_extra_srcs(
name = "sns-wasm_integration_test",
srcs = glob(
Expand Down
Loading

0 comments on commit 09e7929

Please sign in to comment.