ci: update test for celestia-da #39
Conversation
WalkthroughThe code changes involve modifications to the commands used to start a container, retrieve data from a specific endpoint, and execute a command within the container. Additionally, there's a change in handling the retrieved token. These alterations reflect adjustments in the logic and functionality of the code. Changes
Poem
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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (1)
- celestia_test.go (1 hunks)
Additional comments: 4
celestia_test.go (4)
78-78: The command executed has been changed from
celestiatocelestia-da. Ensure that thecelestia-dabinary exists and is correctly placed in the expected directory.83-83: Trimming the token to remove leading and trailing whitespace is a good practice to avoid authentication errors due to formatting issues.
86-86: The
TearDownSuitemethod is responsible for cleaning up the Docker resources. Ensure that thePurgemethod is robust and handles potential errors gracefully.Verification successful
The usage of the
Purgemethod within thecelestia_test.gofile has been identified. It is used as follows:if err := t.pool.Purge(t.resource); err != nil {This indicates that the
Purgemethod is called on apoolobject withresourceas an argument, and error handling is implemented to check if thePurgemethod returns an error.Based on this information, the
TearDownSuitemethod appears to handle potential errors from thePurgemethod gracefully by checking for an error return value.* 86-86: The `getRPCAddress` method constructs the RPC address using the port from the Docker resource. Verify that the port number corresponds to the correct service and that there are no hard-coded values that could conflict with dynamic port assignment.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Verify the behavior of the Purge method in the dockertest library. rg --type go 'func \(.*Pool\) Purge'Length of output: 38
Script:
#!/bin/bash # Search for usage of the Purge method within the repository to understand how it's being handled. rg --type go '(\.Purge)'Length of output: 93
Verification successful
The output from the
rgcommand shows two instances whereGetPortis used incelestia_test.go:
http.Get(fmt.Sprintf("http://localhost:%s/balance", resource.GetPort("26659/tcp")))fmt.Sprintf("http://localhost:%s", t.resource.GetPort("26658/tcp"))Both lines are using dynamic port assignment with the
GetPortmethod, which retrieves the mapped port for a given service specified by the"port/tcp"string. The ports26659/tcpand26658/tcpare used, which suggests that these are the correct services being referenced, and there are no hard-coded values that could conflict with dynamic port assignment.Based on this information, the
getRPCAddressmethod correctly constructs the RPC address using the port from the Docker resource, and there are no issues with hard-coded values.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Verify that the port number used in getRPCAddress corresponds to the correct service. rg --type go 'GetPort'Length of output: 231
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #39 +/- ##
=======================================
Coverage 71.52% 71.52%
=======================================
Files 2 2
Lines 144 144
=======================================
Hits 103 103
Misses 28 28
Partials 13 13 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (1)
- celestia_test.go (4 hunks)
Additional comments: 5
celestia_test.go (5)
48-48: The container version has been updated from "4ecd750" to "v0.12.5". Ensure that this version is compatible with the rest of the test suite and the expected environment.
57-57: The endpoint for retrieving data has been changed from
/balanceto/header/10. Confirm that this endpoint is correctly implemented and that the test is now aligned with the current state of the network.66-66: The error message check for "header: given height is from the future" has been added. This seems to be a specific error handling case. Ensure that this is the correct and only error message that should be handled here, and that it's properly documented.
78-78: The command executed within the container has been changed from using
celestiatocelestia-da. This change should be verified to ensure that thecelestia-dabinary exists and functions as expected within the container.Verification successful
The Dockerfile confirms that the
celestia-dabinary is included in the container image, as it is copied to/bin/celestia-daduring the build process. This supports the change made in thecelestia_test.gofile to execute thecelestia-dacommand within the container.* 83-83: The token is now being trimmed of any white spaces before use. This is a good practice to avoid issues with extra white spaces in tokens. Ensure that this change does not affect any other parts of the system that may expect the token in its previous form.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Verify that the `celestia-da` binary exists within the container and functions as expected. # This might involve checking the container's filesystem or running a command within the container. # Example pseudo-command: # docker exec <container-id> ls /bin/celestia-daLength of output: 0
Script:
#!/bin/bash # Search for Dockerfile or scripts that might be used to build the container image. # This will help us verify if the `celestia-da` binary is included in the container. # Find Dockerfile or relevant build scripts fd 'Dockerfile|build.*\.(sh|bash)' --exec cat {}Length of output: 1437
Overview
This PR fixes an issue with the test suite where the auth token was not being passed correctly, also updates test suite to use
celestia-dainstead ofcelestia.The tests were failing due to:
so replaced the
/balancecheck with/header/10Checklist
Summary by CodeRabbit
Summary by CodeRabbit
celestia-dacommand, ensuring cleaner token handling.