fix(e2e): compatibility e2e test fixes #5761
Conversation
| ], | ||
| "chain-b": [ | ||
| "main", | ||
| "v8.1.0" |
There was a problem hiding this comment.
This will not work until the tag is created, but I added it already anyway, since I am not planning to run this workflow before the release.
| @@ -0,0 +1,18 @@ | |||
| { | |||
There was a problem hiding this comment.
I added new files for compatibility testing of unordered ICA channels.
| @@ -30,6 +30,6 @@ jobs: | |||
| - name: Build image | |||
| run: | | |||
| # remove any `/` characters from the docker tag and replace them with a - | |||
| docker_tag="$(echo $GIT_TAG | sed 's/[^a-zA-Z0-9]/-/g')" | |||
| docker_tag="$(echo $GIT_TAG | sed 's/[^a-zA-Z0-9\.]/-/g')" | |||
There was a problem hiding this comment.
I added the . as an allowed character in docker image tags.
|
Compatibility tests green for |
crodriguezvega
requested review from
colin-axner,
AdityaSripal,
charleenfei,
damiannolan,
chatton and
DimitrisJim
as code owners
|
I'm currently looking at moving the conditionals for unknown fields to ref: https://github.com/cosmos/ibc-go/blob/main/e2e/testsuite/sanitize/messages.go#L34 edit: Took one and ran through it locally, seems to work 👍🏻 I'll push a commit soon ™️ |
|
Changes included in c949d5b. I think it significantly reduced the diff. I ran locally using the regular |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #5761 +/- ##
==========================================
+ Coverage 81.49% 81.54% +0.05%
==========================================
Files 199 199
Lines 15203 15225 +22
==========================================
+ Hits 12389 12415 +26
+ Misses 2348 2344 -4
Partials 466 466
|
There was a problem hiding this comment.
Let me know if the changes look good to you @crodriguezvega! I'll drop my approval on this PR, happy to get it merged.
|
Thank you so much for the improvements, @damiannolan! Changes look great, I triggered a compatibility test run here. I am happy to merge if they all get 🟢. |
There was a problem hiding this comment.
looks good, great savings on the slick filtering before broadcasting @damiannolan!
I see some failures in gov/groups tests, due to the nested messaging. I think we can sanitize before putting into the proposals also. I can look into pushing a fix later if nobody gets around it. edit: nevermind, nice one @crodriguezvega, lets see if that does the trick :) |
|
ICA tests with gov and groups failed, but I added sanitation of the messages contained in the proposal now, so I will trigger another run. |
|
Test run was green, will merge as soon as I have a chance with updating the branch all the time!! |
Description
closes: #XXXX
Commit Message / Changelog Entry
see the guidelines for commit messages. (view raw markdown for examples)
Before we can merge this PR, please make sure that all the following items have been
checked off. If any of the checklist items are not applicable, please leave them but
write a little note why.
docs/) or specification (x/<module>/spec/).godoccomments.Files changedin the Github PR explorer.Codecov Reportin the comment section below once CI passes.