Personal ledger #1766
Closed
Personal ledger #1766
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Contributor
## Walkthrough
Multiple testing-related directories and files were entirely removed, including load testing Dockerfiles, client libraries for various API versions, integration test suites, modules, and helpers. Configuration files for linting, building, and environment setup were also deleted. The removals encompass benchmarks, integration tests, load testing clients, and related infrastructure.
## Changes
| File(s) | Change Summary |
|---------------------------------------------------------------------------------------------|-----------------------------------------------------------------------------------------------------------------------|
| tests/loadtesting/** | Entire load testing directory deleted, including Dockerfile, client libraries (versions 1.3, 1.4, 1.5, 1.7), tests, configs, and scripts. |
| tests/integration/** | Entire integration test suite deleted, including Earthfiles, modules (auth, ledger, orchestration, payments, reconciliation, search, wallets, webhooks), helpers, internal framework, and test suites. |
| tests/benchmarks/** | Entire benchmarks directory deleted, including scripts, extension code, Earthfiles, and configuration files. |
| .editorconfig, .earthlyignore, .envrc, .gitignore, LICENSE, README.md, Earthfile, base.Dockerfile, tests/loadtesting/.babelrc, tests/loadtesting/.eslintrc.js, tests/loadtesting/.gitignore, tests/loadtesting/.yarnclean, tests/loadtesting/webpack.config.js, tests/benchmarks/scripts/webpack.config.js, releases/.gitignore, tests/benchmarks/.gitignore, tests/benchmarks/scripts/.gitignore | Various configuration and ignore files deleted, removing build, linting, environment, and version control ignore settings. |Possibly related PRs
Poem
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
tests/loadtesting/Dockerfile (1)
1-13: Consider multi-stage build to slim down the final image
Currently, build dependencies (Node, yarn, source files) remain in the final image. A multi-stage build could compile theledgerbinary in a builder stage and copy only the runtime artifacts into a minimal base image with k6. This reduces image size and attack surface.Let me know if you’d like a template for a two-stage Dockerfile.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
docker-compose.ymlis excluded by!**/*.ymlservice.ymlis excluded by!**/*.yml
📒 Files selected for processing (1)
tests/loadtesting/Dockerfile(1 hunks)
tests/loadtesting/Dockerfile
Outdated
| RUN yarn install | ||
| RUN yarn run build | ||
| ENTRYPOINT ["/usr/bin/k6"] | ||
| ENTRYPOINT ["/app/ledger"] |
Contributor
There was a problem hiding this comment.
ENTRYPOINT references a non-existent path
The ENTRYPOINT is set to /app/ledger, but this Dockerfile never copies or builds a ledger binary into /app/ledger. At runtime, the container will fail with no such file or directory.
Please ensure that the ledger executable is produced (or copied) into /app/ledger. For example, if your build outputs ledger to dist/ledger, you might add:
RUN yarn run build
+COPY dist/ledger /app/ledger
+RUN chmod +x /app/ledgeror adjust the ENTRYPOINT to point at the correct location of the built binary.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents (early access)
In tests/loadtesting/Dockerfile at line 12, the ENTRYPOINT is set to
/app/ledger, but the Dockerfile does not copy or build the ledger binary into
that path, causing runtime failure. Fix this by either adding a step in the
Dockerfile to copy the built ledger binary (e.g., from dist/ledger) into
/app/ledger or by changing the ENTRYPOINT to point to the actual location of the
ledger executable inside the container.
tests/loadtesting/Dockerfile
Outdated
| RUN yarn run build | ||
| ENTRYPOINT ["/usr/bin/k6"] | ||
| ENTRYPOINT ["/app/ledger"] | ||
| CMD ["run", "tests/benchmarks/scripts/benchs/ledger-v2.js"] |
Contributor
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Verify benchmark script path under /k6 working directory
You’ve set WORKDIR /k6, so CMD ["run", "tests/benchmarks/scripts/benchs/ledger-v2.js"] will resolve to /k6/tests/benchmarks/.... Confirm that this script path exists in your repo and is included in the image.
You can run locally to check before publishing:
or
🏁 Script executed:
rg --files-with-matches 'tests/benchmarks/scripts/benchs/ledger-v2.js'Length of output: 99
🏁 Script executed:
docker build -t ledger-test . && docker run --rm ledger-test ls -R /k6/tests/benchmarks/scripts/benchsLength of output: 128
🏁 Script executed:
#!/bin/bash
# Verify if ledger-v2.js exists anywhere in the repository
rg --files-with-matches 'ledger-v2.js'Length of output: 94
Fix invalid benchmark script path in Dockerfile
The CMD in tests/loadtesting/Dockerfile is pointing at a non‐existent file—no tests/benchmarks/scripts/benchs/ledger-v2.js was found in the repo. Please update the path or add the missing script.
Locations to update:
- tests/loadtesting/Dockerfile line 13
- CMD ["run", "tests/benchmarks/scripts/benchs/ledger-v2.js"] + CMD ["run", "<correct/path>/ledger-v2.js"]
Next steps:
- Verify where
ledger-v2.jsactually lives (or should be generated) and adjust the Docker CMD accordingly. - If the script is missing, add it under the intended directory (
tests/benchmarks/scripts/benchs/) so it’s included in the image.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents (early access)
In tests/loadtesting/Dockerfile at line 13, the CMD references a benchmark
script path tests/benchmarks/scripts/benchs/ledger-v2.js that does not exist in
the image or repository. Verify the correct location of ledger-v2.js or where it
should be generated, then update the CMD to point to the valid path relative to
the WORKDIR /k6. If the script is missing, add it to
tests/benchmarks/scripts/benchs/ so it is included in the Docker image build.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
No description provided.