Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Run bridges zombienet tests on CI #2439
Run bridges zombienet tests on CI #2439
Changes from 91 commits
c6f8e61
1e369ee
4c06584
58e9981
da2bd72
baf85c9
9c921c8
5e5c72b
5ccf7fe
dd7b631
9f617dc
c2327a2
97b7666
638924c
8653c05
0ebc852
81c9ec6
065a69e
4afa6a8
fcc4216
7172580
7393a97
54497f0
e0382ea
8e20872
b050f1a
3230786
d107297
3022c82
f19668c
83d2177
ab99318
c562cd9
f2cb940
8a5ace0
8a95a9e
8c4a7b2
d79a43b
1156753
cb127c9
46b13f7
1f0e267
14ac859
b0c8cf6
a984032
accd8fc
fd32ee7
8e3b76d
67d0f16
adad841
39ca333
6a93477
82f8e64
3a71ba6
c3ebf00
e02121c
960558f
92d3b7f
49d3090
88d4c00
e9b058d
1e2b3b6
ead14d2
f581afa
911de8f
34d894f
2d2a616
0a25a83
d61b5bc
228bc1b
cbe832d
7cb9189
c1743e8
1ce62ec
ab42c1d
523c0a4
96111d0
558a9ee
c12391c
db93f6b
99e0519
811afb3
43c7c04
6df5381
d57e183
3aafccb
0791aae
370b4ce
86906f2
c4ba86f
9b36e5c
327ad7c
78bbc70
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I moved the image definition to
.zombienet-ref
in this pr https://github.com/paritytech/polkadot-sdk/pull/2396/files#diff-037ea159eb0a7cb0ac23b851e66bee30fb838ee8d0d99fa331a1ba65283d37f7L33, so here you will not have the env var defined. I think you can extend from that ref but we should check.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems a bit too specific to our use case. I would try to make this more generic. Maybe something like
adding
${EXTRA_BUILDAH_ARGS}
here and definingEXTRA_BUILDAH_ARGS = --build-arg ZOMBIENET_IMAGE=...
inbuild-push-image-bridges-zombienet-tests
if possible.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I prefer to keep meaningful naming here - we could wrap everything in the
EXTRA_BUILDAH_ARGS
, making it hard to find the origins. But I could change if there's a collective agreement re thatThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: Would be nice if this logs could have a more suggestive name
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, that's a good idea. But now there are just three log files matching this criteria, so not a big deal imo. When I/we will be adding more tests (hope to do that soon) it'll make sense to give meaningful names to those log files. Let's do it later in a separate PR
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: These names seem a bit too verbose. I would try to reduce them a bit. For example something like
ASSET_HUB_ROCOCO_BINARY_PATH
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
They're already used by our scripts - I'm just setting values here :) Actually, I think we don't need this
_FOR_ASSET_HUB_ROCOCO
anymore (right, @bkontur?) - let's remove it in a future PRs