-
Notifications
You must be signed in to change notification settings - Fork 289
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
fix: e2e tests #3487
fix: e2e tests #3487
Conversation
5ff327d
to
2f32294
Compare
WalkthroughThe recent changes introduce a new GitHub Actions workflow for end-to-end (E2E) testing, refactor the existing nightly workflow to use external job configurations, and update test-related Go files to streamline the testing process. These updates aim to enhance the reliability and maintainability of the E2E tests, addressing issues with test execution and environment setup. Changes
Assessment against linked issues
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
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.
[question] out of curiosity how was the git tag issue fixed?
Co-authored-by: Rootul P <[email protected]>
Co-authored-by: Rootul P <[email protected]>
Co-authored-by: Rootul P <[email protected]>
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.
Actionable comments posted: 3
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.
👍
@@ -40,7 +40,7 @@ func main() { | |||
|
|||
if !specificTestFound { | |||
logger.Println("No particular test specified. Running all tests.") | |||
logger.Println("go run ./test/e2e <test_name> to run a specific test") | |||
logger.Println("make test-e2e <test_name> to run a specific test") | |||
logger.Printf("Valid tests are: %s\n\n", getTestNames(tests)) |
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: Does this need to be printed? Doesn't each test usually print their names upon start?
} | ||
|
||
kr, err := testNet.CreateAccount("alice", 1e12, "") | ||
testnet.NoError("failed to create account", err) |
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 for future:
if we move files, ideally (I haven't always done this!) we only move them and not change anything to reduce the effort to see what actually changed. in this case it looked like it was just some comments
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 I tend to agree but since it was only minor changes I applied suggestions
Overview
Fixes #3453
I encountered a lot of flakiness while running knuu tests in ci(view logs in issue description) to resolve them i added a few workarounds with @smuu which broke other things in our existing suite. those changes are now reverted since the tests started passing on main again after scaling some of the clusters that were stuck. DevOps is working on addressing flakiness on knuu.
E2E tests are green: https://github.com/celestiaorg/celestia-app/actions/runs/9179315870/job/25241185362?pr=3487
My changes: