chore(IDX): introduce the pocket_ic_test macro #3268
Conversation
This introduces the `pocket_ic_test` macro exported from `//bazel:pocket-ic-tests.bzl`. It abstracts over depending on the `//rs/pocket_ic_server:pocket-ic-server` and setting the `requires-network` hack to work around the `Operation not permitted` error on darwin.
basvandijk
added
the
CI_ALL_BAZEL_TARGETS
github-actions
bot
added
@finint
@nns-team
@pocket-ic
@boundary-node
@cross-chain-team
@idx
labels
| @@ -92,3 +92,35 @@ pocket_ic_mainnet_test = rule( | |||
| executable = True, | |||
| test = True, | |||
| ) | |||
|
|
|||
| def pocket_ic_test(name, test_rule, **kwargs): | |||
There was a problem hiding this comment.
I'm not too fond of small macros that just wrap a target and inject dependencies; I find they tend to add complexity and obfuscate what is really happening, making everything harder to follow and debug, especially when e.g. the test_rule needs to be specified.
Is the goal here mostly to avoid repetition? Do we have a ticket we can link to re. the sandbox issue on Darwin?
There was a problem hiding this comment.
Context is in this thread. I would like to have a central place where we do work-arounds for that sandbox issue on darwin and not scatter it all over our code base.
There was a problem hiding this comment.
I see; agree it's a good idea to have a central place for the workaround but I don't think this requires implicitly injecting pocket-ic. How about modifying rust_ic_test to add requires-network if pocket-ic is a dependency?
Also, can you add a ticket so that we can track the issue upstream and remove the workaround once it's fixed?
There was a problem hiding this comment.
How about modifying rust_ic_test to add requires-network if pocket-ic is a dependency?
That seems rather hacky and an implicit rule like that could lead to surprises. Also it's not just rust_ic_test that needs to be modified but also rust_test, rust_test_suite and rust_test_suite_with_extra_srcs. Or do they all resolve back to rust_test?
can you add a ticket so that we can track the issue upstream and remove the workaround once it's fixed?
There was a problem hiding this comment.
Sorry, I meant a ticket upstream on bazel! With just an internal ticket I doubt we'll follow up & make progress on this
There was a problem hiding this comment.
This seems like a similar issue: bazelbuild/bazel#14828. Let me study that a bit more later.
There was a problem hiding this comment.
There seems to be a work around by using IPv6 instead of IPv4. @mraszyk maybe that's something you can look into.
This introduces the
pocket_ic_testmacro exported from//bazel:pocket-ic-tests.bzl. It abstracts over depending on the//rs/pocket_ic_server:pocket-ic-serverand setting therequires-networkhack to work around theOperation not permittederror on darwin.