-
Notifications
You must be signed in to change notification settings - Fork 121
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
Small fixes after recent PRs #1116
Conversation
We actually want to run the litd itests on the staging branch that has the custom channel tests. We also can speed up the tests by removing the yarn build for the static files, since we don't need those for the tests (an empty index.html just needs to exist for the build to succeed).
Pull Request Test Coverage Report for Build 10715230733Details
💛 - Coveralls |
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.
Lgtm! thanks for the fixes
for some reason my make lint
run didn't catch these yesterday
uses: actions/checkout@v3 | ||
with: | ||
repository: lightninglabs/lightning-terminal | ||
ref: ${{ env.LITD_ITEST_BRANCH }} |
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.
not sure if related, but there's a tiny misbehavior according to an itest
https://github.com/lightninglabs/taproot-assets/actions/runs/10715230733/job/29710290030?pr=1116#step:4:52159
we get a "status completed" instead of "proofs transferred"
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.
Hmm, that seems to be a GitHub only flake... I ran this test a bunch of time locally and it never failed for me...
Going to re-run CI and just hope it goes away.
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.
Man, this has been driving me bananas! I had something similar way back and never understood why that happens on GitHub.
The weird thing is that in my case when the test ran on Github it expected a different state than what the test was actually supposed to be expecting, and I think that's the case here as well, right?
The test expects SendStateWaitTxConf
taproot-assets/itest/psbt_test.go
Lines 1148 to 1152 in 6f4d65f
AssertSendEvents( | |
t.t, scriptKey1Bytes, sendEvents, | |
tapfreighter.SendStateWaitTxConf, | |
tapfreighter.SendStateComplete, | |
) |
But according to the run on GitHub the test expects SendStateTransferProofs
which makes no sense whatsoever:
https://github.com/lightninglabs/taproot-assets/actions/runs/10715230733/job/29710290030?pr=1116#step:4:52158
Maybe an enumeration is off by one?
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.
The enum values passed into AssertSendEvents()
is a "from" and "to". So it's expecting all events from SendStateWaitTxConf
in order up until SendStateComplete
. And SendStateTransferProofs
just happens to be an in-between one.
So I think it's a pure timing issue, perhaps because GitHub is slower than the local machine.
Fixes formatting of build tag excluded files the linter didn't pick up on.
And fixes the new itest to actually run against the correct
litd
branch (I missed that during review). At the same time we slightly optimize the litd itest build.