CI | Mint Integration with NooBaa #9123
Conversation
## Walkthrough
This update integrates Mint system tests into the NooBaa CI pipeline by adding new GitHub Actions workflows, Makefile targets, scripts, and documentation. It generalizes system test utilities for broader reuse, introduces constant definitions for Mint testing, and provides automation for both containerized and non-containerized test environments.
## Changes
| File(s) | Change Summary |
|------------------------------------------------------------------------------------------|-----------------------------------------------------------------------------------------------------------------------------|
| .github/workflows/mint-nc-tests.yaml, .github/workflows/mint-tests.yaml | Added new GitHub Actions workflows for Mint and NC Mint tests, triggered via workflow calls, automating test execution. |
| .github/workflows/run-pr-tests.yaml | Added jobs to invoke the new Mint and NC Mint test workflows after building the NooBaa image. |
| Makefile | Added `test-mint` and `test-nc-mint` targets; introduced helper function for Docker network disconnect; refactored cleanup. |
| docs/CI & Tests/Mint.md | New documentation describing Mint test integration, workflows, and local/manual execution instructions. |
| src/endpoint/s3/s3_errors.js | Minor change: removed trailing period from `NoSuchBucket` error message. |
| src/test/system_tests/external_tests_utils.js | Refactored and generalized test utility functions for account/bucket creation and key retrieval; improved parameterization. |
| src/test/system_tests/mint/configure_mint.js | New script to configure Mint test environment and create test accounts dynamically. |
| src/test/system_tests/mint/mint_constants.js | New module defining constants and credentials for Mint system tests. |
| src/test/system_tests/mint/run_mint_on_test_container.sh, run_nc_mint_on_test_container.sh | New shell scripts to automate Mint and NC Mint test setup and execution in test containers. |
| src/test/system_tests/warp/configure_warp.js, run_warp.js | Updated to use generalized system test utilities and parameterized account/bucket setup. |
## Sequence Diagram(s)
```mermaid
sequenceDiagram
participant GitHub Actions
participant NooBaa Core Repo
participant Artifact Storage
participant Test Container
GitHub Actions->>NooBaa Core Repo: Checkout source code
GitHub Actions->>Artifact Storage: Download noobaa-tester artifact
GitHub Actions->>Test Container: Load Docker image from artifact
GitHub Actions->>Test Container: Run Mint/NC Mint test script
Test Container->>NooBaa Core Repo: Run Makefile target (test-mint/test-nc-mint)
Test Container->>Test Container: Setup environment, configure accounts, run testsPossibly related PRs
Suggested labels
Suggested reviewers
|
romayalon
force-pushed
the
romy-ci-mint
branch
11 times, most recently
from
f3b0ad3 to
b2d01da
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (5)
src/test/system_tests/mint/mint_constants.js (1)
1-2: Remove redundant 'use strict' directive.Static analysis correctly identifies that the 'use strict' directive is redundant in ES modules, as they are automatically in strict mode.
Apply this fix:
-/* Copyright (C) 2022 NooBaa */ -'use strict'; +/* Copyright (C) 2022 NooBaa */.github/workflows/run-pr-tests.yaml (1)
44-50: Fix indentation and approve mint test integration.The new mint test jobs are properly integrated into the CI workflow, following the established pattern of depending on
build-noobaa-image. However, static analysis correctly identifies indentation issues.Apply this fix for proper YAML indentation:
- mint-tests: - needs: build-noobaa-image - uses: ./.github/workflows/mint-tests.yaml + mint-tests: + needs: build-noobaa-image + uses: ./.github/workflows/mint-tests.yaml - mint-nc-tests: - needs: build-noobaa-image - uses: ./.github/workflows/mint-nc-tests.yaml + mint-nc-tests: + needs: build-noobaa-image + uses: ./.github/workflows/mint-nc-tests.yamlThe integration itself is well-designed and consistent with the existing test job patterns.
src/test/system_tests/mint/configure_mint.js (1)
2-2: Remove redundant "use strict" directive.The "use strict" directive is redundant in JavaScript modules as they are automatically in strict mode.
-"use strict";docs/CI & Tests/Mint.md (2)
34-35: Fix markdown list style consistency.The markdown linter flags inconsistent list styles. Use dashes consistently for unordered lists.
-* [Mint Tests](../../.github/workflows/mint-tests.yaml) - Based on NooBaa Tester image, runs Mint on standard NooBaa (db configuration). -* [Mint NC Tests](../../.github/workflows/mint-nc-tests.yaml) - Based on NooBaa Tester image, runs Mint on non-containerized NooBaa (ConfigFS configuration). +- [Mint Tests](../../.github/workflows/mint-tests.yaml) - Based on NooBaa Tester image, runs Mint on standard NooBaa (db configuration). +- [Mint NC Tests](../../.github/workflows/mint-nc-tests.yaml) - Based on NooBaa Tester image, runs Mint on non-containerized NooBaa (ConfigFS configuration).-* `make run-mint` - Based on NooBaa Tester image, runs Mint on standard NooBaa (db configuration). -* `make run-nc-mint` - Based on NooBaa Tester image, runs Mint on non-containerized NooBaa (ConfigFS configuration). +- `make run-mint` - Based on NooBaa Tester image, runs Mint on standard NooBaa (db configuration). +- `make run-nc-mint` - Based on NooBaa Tester image, runs Mint on non-containerized NooBaa (ConfigFS configuration).Also applies to: 42-43
54-56: Add language specification to fenced code block.The markdown linter flags missing language specification for the fenced code block.
-``` +```bash docker run -e SERVER_ENDPOINT=<noobaa-endpoint-address>:<noobaa-endpoint-port> -e ACCESS_KEY=<pre-existing-account-access-key> -e SECRET_KEY=<pre-existing-account-secret-key> -e ENABLE_HTTPS=0 minio/mint <sdk-or-tool-name></blockquote></details> </blockquote></details> <details> <summary>📜 Review details</summary> **Configuration used: .coderabbit.yaml** **Review profile: CHILL** **Plan: Pro** <details> <summary>📥 Commits</summary> Reviewing files that changed from the base of the PR and between ec3d16c56c44882207c4fc28d4c6eff304a6def2 and a5774ef23ea7d1e4d766bbfd1b7cde0760fd24e1. </details> <details> <summary>📒 Files selected for processing (13)</summary> * `.github/workflows/mint-nc-tests.yaml` (1 hunks) * `.github/workflows/mint-tests.yaml` (1 hunks) * `.github/workflows/run-pr-tests.yaml` (1 hunks) * `Makefile` (9 hunks) * `docs/CI & Tests/Mint.md` (1 hunks) * `src/endpoint/s3/s3_errors.js` (1 hunks) * `src/test/system_tests/external_tests_utils.js` (4 hunks) * `src/test/system_tests/mint/configure_mint.js` (1 hunks) * `src/test/system_tests/mint/mint_constants.js` (1 hunks) * `src/test/system_tests/mint/run_mint_on_test_container.sh` (1 hunks) * `src/test/system_tests/mint/run_nc_mint_on_test_container.sh` (1 hunks) * `src/test/system_tests/warp/configure_warp.js` (2 hunks) * `src/test/system_tests/warp/run_warp.js` (2 hunks) </details> <details> <summary>🧰 Additional context used</summary> <details> <summary>📓 Path-based instructions (1)</summary> <details> <summary>`src/test/**/*.*`: Ensure that the PR includes tests for the changes. </summary> > `src/test/**/*.*`: Ensure that the PR includes tests for the changes. > ⚙️ Source: CodeRabbit Configuration File List of files the instruction was applied to: - `src/test/system_tests/warp/run_warp.js` - `src/test/system_tests/mint/run_mint_on_test_container.sh` - `src/test/system_tests/mint/configure_mint.js` - `src/test/system_tests/mint/mint_constants.js` - `src/test/system_tests/mint/run_nc_mint_on_test_container.sh` - `src/test/system_tests/warp/configure_warp.js` - `src/test/system_tests/external_tests_utils.js` </details> </details><details> <summary>🧠 Learnings (5)</summary> <details> <summary>📓 Common learnings</summary>Learnt from: nadavMiz
PR: #9099
File: .github/workflows/run-pr-tests.yaml:135-136
Timestamp: 2025-06-19T11:03:49.934Z
Learning: In the noobaa-core project, themake testercommand also callsmake noobaadue to makefile target dependencies, so runningmake testerwill build both the noobaa and tester images.</details> <details> <summary>.github/workflows/run-pr-tests.yaml (2)</summary>Learnt from: nadavMiz
PR: #9099
File: .github/workflows/run-pr-tests.yaml:135-136
Timestamp: 2025-06-19T11:03:49.934Z
Learning: In the noobaa-core project, themake testercommand also callsmake noobaadue to makefile target dependencies, so runningmake testerwill build both the noobaa and tester images.Learnt from: nadavMiz
PR: #9099
File: .github/workflows/run-pr-tests.yaml:52-70
Timestamp: 2025-06-19T10:01:46.933Z
Learning: In the noobaa-core repository, branch names don't contain forward slashes, so Docker tag sanitization is not needed in GitHub Actions workflows. Manual full builds and automated builds should use consistent tag naming to avoid mismatches.</details> <details> <summary>src/test/system_tests/mint/run_mint_on_test_container.sh (1)</summary>Learnt from: nadavMiz
PR: #9099
File: .github/workflows/run-pr-tests.yaml:135-136
Timestamp: 2025-06-19T11:03:49.934Z
Learning: In the noobaa-core project, themake testercommand also callsmake noobaadue to makefile target dependencies, so runningmake testerwill build both the noobaa and tester images.</details> <details> <summary>src/test/system_tests/mint/run_nc_mint_on_test_container.sh (1)</summary>Learnt from: nadavMiz
PR: #9099
File: .github/workflows/run-pr-tests.yaml:135-136
Timestamp: 2025-06-19T11:03:49.934Z
Learning: In the noobaa-core project, themake testercommand also callsmake noobaadue to makefile target dependencies, so runningmake testerwill build both the noobaa and tester images.</details> <details> <summary>Makefile (2)</summary>Learnt from: nadavMiz
PR: #9099
File: .github/workflows/run-pr-tests.yaml:135-136
Timestamp: 2025-06-19T11:03:49.934Z
Learning: In the noobaa-core project, themake testercommand also callsmake noobaadue to makefile target dependencies, so runningmake testerwill build both the noobaa and tester images.Learnt from: nadavMiz
PR: #9099
File: .github/workflows/run-pr-tests.yaml:52-70
Timestamp: 2025-06-19T10:01:46.933Z
Learning: In the noobaa-core repository, branch names don't contain forward slashes, so Docker tag sanitization is not needed in GitHub Actions workflows. Manual full builds and automated builds should use consistent tag naming to avoid mismatches.</details> </details><details> <summary>🧬 Code Graph Analysis (3)</summary> <details> <summary>src/test/system_tests/warp/run_warp.js (1)</summary><blockquote> <details> <summary>src/test/system_tests/external_tests_utils.js (1)</summary> * `require` (6-6) </details> </blockquote></details> <details> <summary>src/test/system_tests/mint/mint_constants.js (2)</summary><blockquote> <details> <summary>src/test/system_tests/external_tests_utils.js (2)</summary> * `SensitiveString` (5-5) * `require` (6-6) </details> <details> <summary>src/test/system_tests/mint/configure_mint.js (2)</summary> * `require` (7-7) * `require` (8-8) </details> </blockquote></details> <details> <summary>src/test/system_tests/warp/configure_warp.js (2)</summary><blockquote> <details> <summary>src/test/system_tests/external_tests_utils.js (2)</summary> * `require` (6-6) * `account_options` (129-129) </details> <details> <summary>src/test/system_tests/warp/warp_constants.js (1)</summary> * `WARP_TEST` (12-42) </details> </blockquote></details> </details><details> <summary>🪛 YAMLlint (1.37.1)</summary> <details> <summary>.github/workflows/run-pr-tests.yaml</summary> [warning] 45-45: wrong indentation: expected 8 but found 4 (indentation) --- [warning] 49-49: wrong indentation: expected 8 but found 4 (indentation) </details> </details> <details> <summary>🪛 GitHub Actions: Run PR Tests</summary> <details> <summary>src/test/system_tests/mint/run_mint_on_test_container.sh</summary> [error] 1-1: Mint tests failed: AWS access key Id provided does not exist in records, causing aws-sdk-java tests to fail with AmazonS3Exception (InvalidAccessKeyId). --- [error] 1-1: Mint tests failed: minio-go test 'MakeBucket' function failed with error 'The AWS access key Id you provided does not exist in our records.' --- [error] 1-1: Mint tests failed: s3cmd test 'test_make_bucket' failed with S3 error 403 (InvalidAccessKeyId): The AWS access key Id you provided does not exist in our records. </details> </details> <details> <summary>🪛 Biome (1.9.4)</summary> <details> <summary>src/test/system_tests/mint/configure_mint.js</summary> [error] 1-2: Redundant use strict directive. The entire contents of JavaScript modules are automatically in strict mode, with no statement needed to initiate it. Safe fix: Remove the redundant use strict directive. (lint/suspicious/noRedundantUseStrict) </details> <details> <summary>src/test/system_tests/mint/mint_constants.js</summary> [error] 1-2: Redundant use strict directive. The entire contents of JavaScript modules are automatically in strict mode, with no statement needed to initiate it. Safe fix: Remove the redundant use strict directive. (lint/suspicious/noRedundantUseStrict) </details> </details> <details> <summary>🪛 markdownlint-cli2 (0.17.2)</summary> <details> <summary>docs/CI & Tests/Mint.md</summary> 34-34: Unordered list style Expected: dash; Actual: asterisk (MD004, ul-style) --- 35-35: Unordered list style Expected: dash; Actual: asterisk (MD004, ul-style) --- 42-42: Unordered list style Expected: dash; Actual: asterisk (MD004, ul-style) --- 43-43: Unordered list style Expected: dash; Actual: asterisk (MD004, ul-style) --- 54-54: Fenced code blocks should have a language specified null (MD040, fenced-code-language) </details> </details> </details> <details> <summary>🔇 Additional comments (19)</summary><blockquote> <details> <summary>src/test/system_tests/mint/mint_constants.js (1)</summary> `24-27`: **Approve the use of SensitiveString for credentials.** Excellent security practice wrapping access and secret keys in `SensitiveString` to prevent accidental logging or exposure of sensitive data. </details> <details> <summary>src/endpoint/s3/s3_errors.js (1)</summary> `335-335`: **Approve error message cleanup for S3 compatibility.** Removing the trailing period from the NoSuchBucket error message improves compatibility with AWS S3 error responses, which is beneficial for Mint testing that validates S3 correctness. </details> <details> <summary>src/test/system_tests/warp/run_warp.js (2)</summary> `11-11`: **Approve import refactoring for code reuse.** Good refactoring to use the generalized `external_tests_utils` module instead of warp-specific utilities, promoting code reuse across different test types. --- `84-84`: **Approve function call update for consistency.** The updated function call to `get_account_access_keys` maintains the same interface while using the generalized utility function, ensuring consistency across test modules. </details> <details> <summary>src/test/system_tests/warp/configure_warp.js (2)</summary> `8-8`: **Approve utility refactoring for deployment flexibility.** Excellent refactoring to use generalized external test utilities that support both containerized and non-containerized deployments, enabling code reuse and consistent test setup patterns. --- `22-26`: **Approve deployment-aware configuration selection.** Well-implemented conditional logic that selects appropriate account and bucket parameters based on the deployment environment. This design enables the same configuration script to work seamlessly across different deployment types while using the generalized system test functions. </details> <details> <summary>.github/workflows/mint-tests.yaml (1)</summary> `1-33`: **LGTM: Well-structured Mint test workflow.** The workflow correctly implements the Mint testing pipeline with proper artifact handling and Docker image loading. The use of `-o tester` override is appropriate to skip rebuilding the tester image since it's already loaded from the downloaded artifact. </details> <details> <summary>src/test/system_tests/mint/configure_mint.js (1)</summary> `10-26`: **LGTM: Proper error handling and configuration logic.** The main function and mint_test_setup function are well-structured with appropriate error handling and logging. The conditional logic for containerized vs non-containerized deployment is correct. </details> <details> <summary>.github/workflows/mint-nc-tests.yaml (1)</summary> `2-34`: **LGTM: Consistent NC Mint test workflow.** The workflow correctly implements the NC (Non-Containerized) Mint testing pipeline, maintaining consistency with the regular Mint tests workflow while using the appropriate `test-nc-mint` target. </details> <details> <summary>src/test/system_tests/mint/run_nc_mint_on_test_container.sh (3)</summary> `1-24`: **Well-structured script setup with proper error handling.** The script follows good practices with error handling (`set -e`), verbose logging (`set -x`), and organized environment variable exports. The hardcoded test credentials are acceptable for this test environment context. --- `27-44`: **Appropriate test environment configuration.** The directory creation and permission setup is well-documented and necessary for allowing test accounts to create buckets. The open permissions (777) are appropriately justified for the test container environment. --- `47-56`: **Dependency scripts exist and are correctly configured** - `./src/deploy/NVA_build/standalone_deploy_nsfs.sh` is present and has executable permissions. - `./src/test/system_tests/mint/configure_mint.js` is present and will be invoked via `node`, so no executability bit is required. No further changes needed. </details> <details> <summary>Makefile (4)</summary> `106-112`: **Well-organized MINT variables section.** The new MINT variables follow existing naming conventions and clearly contain test-only mock credentials. The organization with descriptive comments enhances maintainability. --- `371-383`: **Comprehensive mint test target with proper container lifecycle management.** The `test-mint` target follows established patterns with appropriate network creation, container orchestration, and cleanup. The 90-second sleep provides adequate time for NooBaa initialization before running tests. --- `386-395`: **Appropriate NC mint test target for namespace client testing.** The `test-nc-mint` target correctly omits database dependencies for namespace client testing and uses an appropriate 20-second initialization delay. --- `455-462`: **Excellent refactoring that improves modularity and reusability.** The addition of the `disconnect_container_from_noobaa_network` helper function eliminates code duplication, and the optional container name parameter in `stop_noobaa` enhances flexibility while maintaining backward compatibility. Also applies to: 480-484 </details> <details> <summary>src/test/system_tests/external_tests_utils.js (3)</summary> `57-87`: **Excellent refactoring that generalizes test utilities for broader reuse.** The renaming from warp-specific to generic function names and the introduction of parameter objects significantly improves the flexibility and reusability of these utilities across different system test suites. Also applies to: 110-120 --- `90-104`: **Consistent refactoring pattern for bucket creation functions.** The bucket creation functions now accept flexible parameter objects and maintain clear separation between account and bucket options, improving the API design and usability. Also applies to: 124-135 --- `141-143`: **Clean deployment type detection and properly updated exports.** The `is_containerized_deployment` function provides a clear way to check deployment type, and the exports section correctly reflects all the refactored function names and new additions. Also applies to: 146-151 </details> </blockquote></details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
.github/workflows/mint-tests.yaml (1)
28-34: Over-permissivechmod 777& trailing whitespace
- Setting
777on a log directory inside the runner is unnecessary and violates the principle of least privilege.755(or even700) is sufficient.- YAML-lint flags trailing spaces on line 34 – easy cleanup.
- mkdir -p logs/mint-test-logs - chmod 777 logs/mint-test-logs + mkdir -p logs/mint-test-logs + chmod 755 logs/mint-test-logs
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
.github/workflows/mint-nc-tests.yaml(1 hunks).github/workflows/mint-tests.yaml(1 hunks)Makefile(9 hunks)src/test/system_tests/mint/run_mint_on_test_container.sh(1 hunks)src/test/system_tests/mint/run_nc_mint_on_test_container.sh(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- src/test/system_tests/mint/run_mint_on_test_container.sh
- src/test/system_tests/mint/run_nc_mint_on_test_container.sh
- .github/workflows/mint-nc-tests.yaml
- Makefile
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: nadavMiz
PR: noobaa/noobaa-core#9099
File: .github/workflows/run-pr-tests.yaml:135-136
Timestamp: 2025-06-19T11:03:49.934Z
Learning: In the noobaa-core project, the `make tester` command also calls `make noobaa` due to makefile target dependencies, so running `make tester` will build both the noobaa and tester images.
🪛 YAMLlint (1.37.1)
.github/workflows/mint-tests.yaml
[error] 34-34: trailing spaces
(trailing-spaces)
[warning] 47-47: too many blank lines
(1 > 0) (empty-lines)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: run-package-lock-validation
- GitHub Check: Build Noobaa Image
- GitHub Check: run-jest-unit-tests
.github/workflows/mint-tests.yaml
Outdated
| - name: Download artifact | ||
| uses: actions/download-artifact@v4 | ||
| with: | ||
| name: noobaa-tester | ||
| path: /tmp | ||
|
|
||
| - name: Load image | ||
| run: docker load --input /tmp/noobaa-tester.tar | ||
|
|
There was a problem hiding this comment.
Artifact extraction path is wrong – docker load will fail
actions/download-artifact extracts the files into <path>/<artifact-name>/….
With path: /tmp, the tarball ends up at /tmp/noobaa-tester/noobaa-tester.tar, not /tmp/noobaa-tester.tar.
docker load therefore receives a non-existent file and the job will error out.
- - name: Load image
- run: docker load --input /tmp/noobaa-tester.tar
+ - name: Load image
+ run: docker load --input /tmp/noobaa-tester/noobaa-tester.tar📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| - name: Download artifact | |
| uses: actions/download-artifact@v4 | |
| with: | |
| name: noobaa-tester | |
| path: /tmp | |
| - name: Load image | |
| run: docker load --input /tmp/noobaa-tester.tar | |
| - name: Download artifact | |
| uses: actions/download-artifact@v4 | |
| with: | |
| name: noobaa-tester | |
| path: /tmp | |
| - name: Load image | |
| run: docker load --input /tmp/noobaa-tester/noobaa-tester.tar |
🤖 Prompt for AI Agents
In .github/workflows/mint-tests.yaml around lines 19 to 27, the artifact
extraction path is incorrect because the downloaded artifact is placed in a
subdirectory named after the artifact. Update the docker load command to use the
correct path /tmp/noobaa-tester/noobaa-tester.tar instead of
/tmp/noobaa-tester.tar, or change the download-artifact path to extract directly
to /tmp without the artifact name subfolder so that docker load can find the
tarball and run successfully.
dda4b73 to
1c7d728
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (3)
src/test/system_tests/mint/mint_constants.js (2)
9-10: Ensure hardcoded filesystem paths are created or made configurable.
31-33: Review hardcoded UID/GID values for portability..github/workflows/mint-tests.yaml (1)
25-26: Fix artifact extraction path for docker load.
🧹 Nitpick comments (11)
src/test/system_tests/mint/mint_constants.js (1)
1-2: Remove redundant 'use strict' directive.JavaScript modules are automatically in strict mode, making this directive unnecessary.
-/* Copyright (C) 2022 NooBaa */ -'use strict'; +/* Copyright (C) 2022 NooBaa */.github/workflows/run-pr-tests.yaml (1)
44-50: Fix indentation to match existing jobs.The mint test jobs have incorrect indentation compared to other jobs in the workflow.
- mint-tests: - needs: build-noobaa-image - uses: ./.github/workflows/mint-tests.yaml - - mint-nc-tests: - needs: build-noobaa-image - uses: ./.github/workflows/mint-nc-tests.yaml + mint-tests: + needs: build-noobaa-image + uses: ./.github/workflows/mint-tests.yaml + + mint-nc-tests: + needs: build-noobaa-image + uses: ./.github/workflows/mint-nc-tests.yamlsrc/test/system_tests/mint/configure_mint.js (1)
1-2: Remove redundant 'use strict' directive.JavaScript modules are automatically in strict mode.
-/* Copyright (C) 2016 NooBaa */ -"use strict"; +/* Copyright (C) 2016 NooBaa */.github/workflows/mint-tests.yaml (2)
34-34: Remove trailing spaces.- chmod 777 logs/mint-test-logs - + chmod 777 logs/mint-test-logs +
48-49: Remove extra blank line.retention-days: "1" - -docs/CI & Tests/Mint.md (6)
12-14: Polish the introductory sentence for clarity & grammarMinor language tweaks improve readability and eliminate a “missing determiner” lint warning.
-[Mint](https://github.com/minio/mint) is a testing framework for S3-compatible object storage systems, NooBaa CI runs Mint as correctness/benchmarking and stress tests for the NooBaa system on both containerized and Non Containerized flavors. -Following are the SDKs/tools used in correctness tests. +[Mint](https://github.com/minio/mint) is a testing framework for S3-compatible object-storage systems. +NooBaa CI runs Mint for correctness, benchmarking, and stress tests on both containerized and **non-containerized** deployments. +The following SDKs/tools are covered:
34-36: Align list markers with markdown-lint rules (MD004)
markdownlint-cli2flags the asterisk style. Switching to dash keeps docs lint-clean and consistent with the rest of the repo.-* [Mint Tests](../../.github/workflows/mint-tests.yaml) - Based on NooBaa Tester image, runs Mint on standard NooBaa (db configuration). -* [Mint NC Tests](../../.github/workflows/mint-nc-tests.yaml) - Based on NooBaa Tester image, runs Mint on non-containerized NooBaa (ConfigFS configuration). +- [Mint Tests](../../.github/workflows/mint-tests.yaml) - Based on NooBaa Tester image, runs Mint on standard NooBaa (DB configuration). +- [Mint NC Tests](../../.github/workflows/mint-nc-tests.yaml) - Based on NooBaa Tester image, runs Mint on non-containerized NooBaa (ConfigFS configuration).
41-44: Use dash list markers here as well for consistency-* `make test-mint` - Based on NooBaa Tester image, runs Mint on standard NooBaa (db configuration). -* `make test-nc-mint` - Based on NooBaa Tester image, runs Mint on non-containerized NooBaa (ConfigFS configuration). +- `make test-mint` - Based on NooBaa Tester image, runs Mint on standard NooBaa (DB configuration). +- `make test-nc-mint` - Based on NooBaa Tester image, runs Mint on non-containerized NooBaa (ConfigFS configuration).
54-57: Fix duplicate numbering & clarify step namesTwo “Tab 2” bullets can confuse readers.
-1. Tab 1 - Install NooBaa -2. Tab 2 - Create a NooBaa account. -2. Tab 2 - Run Mint pointing to NooBaa endpoint - +1. Terminal 1 – Install NooBaa +2. Terminal 2 – Create a NooBaa account +3. Terminal 2 – Run Mint pointing to the NooBaa endpoint –
57-60: Add language to fenced code block (MD040)-``` +```bash docker run -e SERVER_ENDPOINT=<noobaa-endpoint-address>:<noobaa-endpoint-http-port> -e ACCESS_KEY=<pre-existing-account-access-key> -e SECRET_KEY=<pre-existing-account-secret-key> -e ENABLE_HTTPS=0 minio/mint <sdk-or-tool-name>--- `68-71`: **Specify language for second code block & line-wrap for readability** ```diff -``` -docker run -e SERVER_ENDPOINT=<noobaa-tester-container-id-or-name>:<noobaa-http-endpoint-port> -e ACCESS_KEY=<pre-existing-account-access-key> -e SECRET_KEY=<pre-existing-account-secret-key> -e ENABLE_HTTPS=0 --network noobaa-net minio/mint <sdk-or-tool-name> -``` +```bash +docker run \ + -e SERVER_ENDPOINT=<noobaa-tester-container-id-or-name>:<noobaa-http-endpoint-port> \ + -e ACCESS_KEY=<pre-existing-account-access-key> \ + -e SECRET_KEY=<pre-existing-account-secret-key> \ + -e ENABLE_HTTPS=0 \ + --network noobaa-net \ + minio/mint <sdk-or-tool-name> +```
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (13)
.github/workflows/mint-nc-tests.yaml(1 hunks).github/workflows/mint-tests.yaml(1 hunks).github/workflows/run-pr-tests.yaml(1 hunks)Makefile(9 hunks)docs/CI & Tests/Mint.md(1 hunks)src/endpoint/s3/s3_errors.js(1 hunks)src/test/system_tests/external_tests_utils.js(4 hunks)src/test/system_tests/mint/configure_mint.js(1 hunks)src/test/system_tests/mint/mint_constants.js(1 hunks)src/test/system_tests/mint/run_mint_on_test_container.sh(1 hunks)src/test/system_tests/mint/run_nc_mint_on_test_container.sh(1 hunks)src/test/system_tests/warp/configure_warp.js(2 hunks)src/test/system_tests/warp/run_warp.js(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (7)
- src/endpoint/s3/s3_errors.js
- src/test/system_tests/warp/configure_warp.js
- src/test/system_tests/warp/run_warp.js
- src/test/system_tests/mint/run_nc_mint_on_test_container.sh
- .github/workflows/mint-nc-tests.yaml
- src/test/system_tests/external_tests_utils.js
- Makefile
🧰 Additional context used
📓 Path-based instructions (1)
`src/test/**/*.*`: Ensure that the PR includes tests for the changes.
src/test/**/*.*: Ensure that the PR includes tests for the changes.
⚙️ Source: CodeRabbit Configuration File
List of files the instruction was applied to:
src/test/system_tests/mint/run_mint_on_test_container.shsrc/test/system_tests/mint/configure_mint.jssrc/test/system_tests/mint/mint_constants.js
🧠 Learnings (4)
📓 Common learnings
Learnt from: nadavMiz
PR: noobaa/noobaa-core#9099
File: .github/workflows/run-pr-tests.yaml:135-136
Timestamp: 2025-06-19T11:03:49.934Z
Learning: In the noobaa-core project, the `make tester` command also calls `make noobaa` due to makefile target dependencies, so running `make tester` will build both the noobaa and tester images.
.github/workflows/mint-tests.yaml (1)
Learnt from: nadavMiz
PR: noobaa/noobaa-core#9099
File: .github/workflows/run-pr-tests.yaml:52-70
Timestamp: 2025-06-19T10:01:46.933Z
Learning: In the noobaa-core repository, branch names don't contain forward slashes, so Docker tag sanitization is not needed in GitHub Actions workflows. Manual full builds and automated builds should use consistent tag naming to avoid mismatches.
.github/workflows/run-pr-tests.yaml (2)
Learnt from: nadavMiz
PR: noobaa/noobaa-core#9099
File: .github/workflows/run-pr-tests.yaml:135-136
Timestamp: 2025-06-19T11:03:49.934Z
Learning: In the noobaa-core project, the `make tester` command also calls `make noobaa` due to makefile target dependencies, so running `make tester` will build both the noobaa and tester images.
Learnt from: nadavMiz
PR: noobaa/noobaa-core#9099
File: .github/workflows/run-pr-tests.yaml:52-70
Timestamp: 2025-06-19T10:01:46.933Z
Learning: In the noobaa-core repository, branch names don't contain forward slashes, so Docker tag sanitization is not needed in GitHub Actions workflows. Manual full builds and automated builds should use consistent tag naming to avoid mismatches.
docs/CI & Tests/Mint.md (1)
Learnt from: nadavMiz
PR: noobaa/noobaa-core#9099
File: .github/workflows/run-pr-tests.yaml:135-136
Timestamp: 2025-06-19T11:03:49.934Z
Learning: In the noobaa-core project, the `make tester` command also calls `make noobaa` due to makefile target dependencies, so running `make tester` will build both the noobaa and tester images.
🧬 Code Graph Analysis (1)
src/test/system_tests/mint/mint_constants.js (2)
src/test/system_tests/external_tests_utils.js (2)
SensitiveString(5-5)require(6-6)src/test/system_tests/mint/configure_mint.js (2)
require(7-7)require(8-8)
🪛 YAMLlint (1.37.1)
.github/workflows/mint-tests.yaml
[error] 34-34: trailing spaces
(trailing-spaces)
[warning] 48-48: too many blank lines
(1 > 0) (empty-lines)
.github/workflows/run-pr-tests.yaml
[warning] 45-45: wrong indentation: expected 8 but found 4
(indentation)
[warning] 49-49: wrong indentation: expected 8 but found 4
(indentation)
🪛 LanguageTool
docs/CI & Tests/Mint.md
[uncategorized] ~12-~12: A determiner appears to be missing. Consider inserting it.
Context: ...inerized and Non Containerized flavors. Following are the SDKs/tools used in correctness ...
(AI_EN_LECTOR_MISSING_DETERMINER)
🪛 markdownlint-cli2 (0.17.2)
docs/CI & Tests/Mint.md
34-34: Unordered list style
Expected: dash; Actual: asterisk
(MD004, ul-style)
35-35: Unordered list style
Expected: dash; Actual: asterisk
(MD004, ul-style)
42-42: Unordered list style
Expected: dash; Actual: asterisk
(MD004, ul-style)
43-43: Unordered list style
Expected: dash; Actual: asterisk
(MD004, ul-style)
57-57: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
68-68: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🪛 Biome (1.9.4)
src/test/system_tests/mint/configure_mint.js
[error] 1-2: Redundant use strict directive.
The entire contents of JavaScript modules are automatically in strict mode, with no statement needed to initiate it.
Safe fix: Remove the redundant use strict directive.
(lint/suspicious/noRedundantUseStrict)
src/test/system_tests/mint/mint_constants.js
[error] 1-2: Redundant use strict directive.
The entire contents of JavaScript modules are automatically in strict mode, with no statement needed to initiate it.
Safe fix: Remove the redundant use strict directive.
(lint/suspicious/noRedundantUseStrict)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: ceph-s3-tests / Ceph S3 Tests
🔇 Additional comments (3)
src/test/system_tests/mint/mint_constants.js (1)
24-27: LGTM: Proper use of SensitiveString for credentials.The mock access and secret keys are correctly wrapped in SensitiveString to prevent accidental logging or exposure.
src/test/system_tests/mint/run_mint_on_test_container.sh (1)
1-54: LGTM: Well-structured environment setup script.The script properly:
- Sets up comprehensive environment variables for NooBaa services
- Uses appropriate PostgreSQL connection limits
- Creates necessary directories with correct permissions
- Follows clear sectioning with descriptive comments
- Uses
set -efor proper error handling.github/workflows/mint-tests.yaml (1)
41-47: LGTM: Correct log upload path.The artifact upload correctly uses
noobaa-core/logs/mint-test-logspath, which matches where the logs are created.
| async function mint_test_setup() { | ||
| console.info('MINT TEST CONFIGURATION:', JSON.stringify(MINT_TEST)); | ||
| const is_containerized = is_containerized_deployment(); | ||
| const account_options = is_containerized ? MINT_TEST.mint_account_params : MINT_TEST.nc_mint_account_params; | ||
| await create_system_test_account(account_options); | ||
| console.info('MINT TEST SETUP DONE'); | ||
| } |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Consider credential propagation for mint tests.
The script creates accounts using mock credentials from constants, but mint tests may need the actual generated credentials. Based on previous feedback, ensure that real AWS credentials are exported to environment variables after account creation.
🏁 Script executed:
#!/bin/bash
# Check if get_account_access_keys function is available in external_tests_utils.js
ast-grep --pattern 'function get_account_access_keys($_) {
$$$
}'
# Check if configure_mint.js handles credential export
rg -A 5 -B 5 "AWS_ACCESS_KEY_ID|AWS_SECRET_ACCESS_KEY" src/test/system_tests/mint/configure_mint.jsLength of output: 1053
🏁 Script executed:
#!/bin/bash
# Locate definition and usage of create_system_test_account to check for credential export
rg -n "create_system_test_account" -A5 -B5 srcLength of output: 6247
🏁 Script executed:
#!/bin/bash
# Extract create_system_test_account implementation to inspect for credential export
sed -n '76,140p' src/test/system_tests/external_tests_utils.js
# Search for any AWS credential exports in external_tests_utils.js
rg -n 'process.env\.(AWS_ACCESS_KEY_ID|AWS_SECRET_ACCESS_KEY)' src/test/system_tests/external_tests_utils.jsLength of output: 2549
Export AWS credentials after account creation in mint tests
The mint_test_setup helper currently creates the test account but never propagates its AWS access keys into the environment, so downstream AWS SDK calls will fail. To fix this, import get_account_access_keys and set AWS_ACCESS_KEY_ID and AWS_SECRET_ACCESS_KEY after create_system_test_account.
• In src/test/system_tests/mint/configure_mint.js, update imports and the mint_test_setup function.
• Verify that your account_options object has a property (e.g. name or email) that get_account_access_keys can use to look up the keys.
--- a/src/test/system_tests/mint/configure_mint.js
+++ b/src/test/system_tests/mint/configure_mint.js
@@ -7,6 +7,7 @@
const { MINT_TEST } = require('./mint_constants');
-const { is_containerized_deployment, create_system_test_account } = require('../external_tests_utils.js');
+const {
+ is_containerized_deployment,
+ create_system_test_account,
+ get_account_access_keys
+} = require('../external_tests_utils.js');
async function mint_test_setup() {
console.info('MINT TEST CONFIGURATION:', JSON.stringify(MINT_TEST));
@@ -23,6 +24,12 @@ async function mint_test_setup() {
const account_options = is_containerized
? MINT_TEST.mint_account_params
: MINT_TEST.nc_mint_account_params;
- await create_system_test_account(account_options);
+ await create_system_test_account(account_options);
+
+ // Propagate AWS credentials for subsequent SDK calls
+ const { access_key, secret_key } =
+ await get_account_access_keys(account_options.name || account_options.email);
+ process.env.AWS_ACCESS_KEY_ID = access_key;
+ process.env.AWS_SECRET_ACCESS_KEY = secret_key;
console.info('MINT TEST SETUP DONE');
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| async function mint_test_setup() { | |
| console.info('MINT TEST CONFIGURATION:', JSON.stringify(MINT_TEST)); | |
| const is_containerized = is_containerized_deployment(); | |
| const account_options = is_containerized ? MINT_TEST.mint_account_params : MINT_TEST.nc_mint_account_params; | |
| await create_system_test_account(account_options); | |
| console.info('MINT TEST SETUP DONE'); | |
| } | |
| // src/test/system_tests/mint/configure_mint.js | |
| const { MINT_TEST } = require('./mint_constants'); | |
| const { | |
| is_containerized_deployment, | |
| create_system_test_account, | |
| get_account_access_keys | |
| } = require('../external_tests_utils.js'); | |
| async function mint_test_setup() { | |
| console.info('MINT TEST CONFIGURATION:', JSON.stringify(MINT_TEST)); | |
| const is_containerized = is_containerized_deployment(); | |
| const account_options = is_containerized | |
| ? MINT_TEST.mint_account_params | |
| : MINT_TEST.nc_mint_account_params; | |
| await create_system_test_account(account_options); | |
| // Propagate AWS credentials for subsequent SDK calls | |
| const { access_key, secret_key } = | |
| await get_account_access_keys(account_options.name || account_options.email); | |
| process.env.AWS_ACCESS_KEY_ID = access_key; | |
| process.env.AWS_SECRET_ACCESS_KEY = secret_key; | |
| console.info('MINT TEST SETUP DONE'); | |
| } |
🤖 Prompt for AI Agents
In src/test/system_tests/mint/configure_mint.js around lines 20 to 26, the
mint_test_setup function creates the test account but does not export the AWS
credentials to the environment, causing downstream AWS SDK calls to fail. To fix
this, import get_account_access_keys at the top of the file, then after
create_system_test_account(account_options), call get_account_access_keys with a
suitable identifier from account_options (like name or email) to retrieve the
AWS keys. Finally, set process.env.AWS_ACCESS_KEY_ID and
process.env.AWS_SECRET_ACCESS_KEY with the retrieved keys to propagate them into
the environment.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
.github/workflows/mint-tests.yaml (1)
19-26: Artifact is unpacked into a sub-directory –docker loadpath is still wrong
actions/download-artifactkeeps the artifact inside a subfolder named after the artifact (/tmp/noobaa-tester/noobaa-tester.tar).
docker load --input /tmp/noobaa-tester.tartherefore fails with “file not found”.- - name: Load image - run: docker load --input /tmp/noobaa-tester.tar + - name: Load image + run: docker load --input /tmp/noobaa-tester/noobaa-tester.tar
🧹 Nitpick comments (5)
.github/workflows/mint-tests.yaml (1)
25-27: Trailing whitespace triggers YAML-lint errorLine 27 ends with a stray space – please trim it to keep the workflow YAML-lint-clean.
- run: docker load --input /tmp/noobaa-tester/noobaa-tester.tar␣ + run: docker load --input /tmp/noobaa-tester/noobaa-tester.tardocs/CI & Tests/Mint.md (4)
12-14: Minor grammar – missing determiner before “containerized”“…on both the containerized and non-containerized flavors.”
34-36: Bullet style violates project markdown-lint ruleProject lint expects dash-style unordered lists. Replace leading
*with-to silence MD004.-* [Mint Tests](../../.github/workflows/mint-tests.yaml) - Based on NooBaa Tester image, runs Mint on standard NooBaa (db configuration). -* [Mint NC Tests](../../.github/workflows/mint-nc-tests.yaml) - Based on NooBaa Tester image, runs Mint on non-containerized NooBaa (ConfigFS configuration). +- [Mint Tests](../../.github/workflows/mint-tests.yaml) - Based on NooBaa Tester image, runs Mint on standard NooBaa (db configuration). +- [Mint NC Tests](../../.github/workflows/mint-nc-tests.yaml) - Based on NooBaa Tester image, runs Mint on non-containerized NooBaa (ConfigFS configuration).
42-44: Same markdown-lint issue for Makefile target list-* `make test-mint` - Based on NooBaa Tester image, runs Mint on standard NooBaa (db configuration). -* `make test-nc-mint` - Based on NooBaa Tester image, runs Mint on non-containerized NooBaa (ConfigFS configuration). +- `make test-mint` - Based on NooBaa Tester image, runs Mint on standard NooBaa (db configuration). +- `make test-nc-mint` - Based on NooBaa Tester image, runs Mint on non-containerized NooBaa (ConfigFS configuration).
57-60: Specify language for fenced code blocks to satisfy MD040-``` +```bash docker run -e SERVER_ENDPOINT=<noobaa-endpoint-address>:<noobaa-endpoint-http-port> -e ACCESS_KEY=<pre-existing-account-access-key> -e SECRET_KEY=<pre-existing-account-secret-key> -e ENABLE_HTTPS=0 minio/mint <sdk-or-tool-name>Apply the same to the block at lines 68-70. </blockquote></details> </blockquote></details> <details> <summary>📜 Review details</summary> **Configuration used: .coderabbit.yaml** **Review profile: CHILL** **Plan: Pro** <details> <summary>📥 Commits</summary> Reviewing files that changed from the base of the PR and between 1c7d728aee540435fba2a53af2d3eea681622896 and c204edb937b5270f1d8cf476e738b52bbf91e0e2. </details> <details> <summary>📒 Files selected for processing (4)</summary> * `.github/workflows/mint-nc-tests.yaml` (1 hunks) * `.github/workflows/mint-tests.yaml` (1 hunks) * `Makefile` (8 hunks) * `docs/CI & Tests/Mint.md` (1 hunks) </details> <details> <summary>🚧 Files skipped from review as they are similar to previous changes (2)</summary> * .github/workflows/mint-nc-tests.yaml * Makefile </details> <details> <summary>🧰 Additional context used</summary> <details> <summary>🧠 Learnings (3)</summary> <details> <summary>📓 Common learnings</summary>Learnt from: nadavMiz
PR: #9099
File: .github/workflows/run-pr-tests.yaml:135-136
Timestamp: 2025-06-19T11:03:49.934Z
Learning: In the noobaa-core project, themake testercommand also callsmake noobaadue to makefile target dependencies, so runningmake testerwill build both the noobaa and tester images.</details> <details> <summary>docs/CI & Tests/Mint.md (1)</summary>Learnt from: nadavMiz
PR: #9099
File: .github/workflows/run-pr-tests.yaml:135-136
Timestamp: 2025-06-19T11:03:49.934Z
Learning: In the noobaa-core project, themake testercommand also callsmake noobaadue to makefile target dependencies, so runningmake testerwill build both the noobaa and tester images.</details> <details> <summary>.github/workflows/mint-tests.yaml (1)</summary>Learnt from: nadavMiz
PR: #9099
File: .github/workflows/run-pr-tests.yaml:52-70
Timestamp: 2025-06-19T10:01:46.933Z
Learning: In the noobaa-core repository, branch names don't contain forward slashes, so Docker tag sanitization is not needed in GitHub Actions workflows. Manual full builds and automated builds should use consistent tag naming to avoid mismatches.</details> </details><details> <summary>🪛 LanguageTool</summary> <details> <summary>docs/CI & Tests/Mint.md</summary> [uncategorized] ~12-~12: A determiner appears to be missing. Consider inserting it. Context: ...inerized and Non Containerized flavors. Following are the SDKs/tools used in correctness ... (AI_EN_LECTOR_MISSING_DETERMINER) </details> </details> <details> <summary>🪛 markdownlint-cli2 (0.17.2)</summary> <details> <summary>docs/CI & Tests/Mint.md</summary> 34-34: Unordered list style Expected: dash; Actual: asterisk (MD004, ul-style) --- 35-35: Unordered list style Expected: dash; Actual: asterisk (MD004, ul-style) --- 42-42: Unordered list style Expected: dash; Actual: asterisk (MD004, ul-style) --- 43-43: Unordered list style Expected: dash; Actual: asterisk (MD004, ul-style) --- 57-57: Fenced code blocks should have a language specified (MD040, fenced-code-language) --- 68-68: Fenced code blocks should have a language specified (MD040, fenced-code-language) </details> </details> <details> <summary>🪛 YAMLlint (1.37.1)</summary> <details> <summary>.github/workflows/mint-tests.yaml</summary> [error] 27-27: trailing spaces (trailing-spaces) --- [warning] 34-34: too many blank lines (2 > 0) (empty-lines) </details> </details> </details> <details> <summary>⏰ Context from checks skipped due to timeout of 90000ms (3)</summary> * GitHub Check: run-package-lock-validation * GitHub Check: Build Noobaa Image * GitHub Check: run-jest-unit-tests </details> <details> <summary>🔇 Additional comments (1)</summary><blockquote> <details> <summary>.github/workflows/mint-tests.yaml (1)</summary> `30-33`: **`make … -o tester` likely does nothing useful** `make -o <file>` marks a *file* as up-to-date, not a target. Passing the phony target name `tester` has no effect and may hide real rebuilds. If the intention is to skip the `tester` prerequisite entirely, use the `--no-print-directory` flag or introduce a dedicated Makefile option. Otherwise just drop `-o tester`. </details> </blockquote></details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (4)
.github/workflows/mint-tests.yaml (1)
25-26: Fix artifact path for docker load command.The
actions/download-artifactaction extracts files to<path>/<artifact-name>/, so the tarball will be at/tmp/noobaa-tester/noobaa-tester.tar, not/tmp/noobaa-tester.tar. This will cause the docker load command to fail.Apply this diff to fix the path:
- - name: Load image - run: docker load --input /tmp/noobaa-tester.tar + - name: Load image + run: docker load --input /tmp/noobaa-tester/noobaa-tester.tarsrc/test/system_tests/mint/configure_mint.js (1)
20-26: Export AWS credentials after account creation.The script creates the test account but doesn't export the AWS credentials to environment variables, which will cause downstream AWS SDK calls to fail. The credentials need to be propagated to
process.envafter account creation.Apply this diff to fix the credential propagation:
-const { is_containerized_deployment, create_system_test_account } = require('../external_tests_utils.js'); +const { is_containerized_deployment, create_system_test_account, get_account_access_keys } = require('../external_tests_utils.js'); async function mint_test_setup() { console.info('MINT TEST CONFIGURATION:', JSON.stringify(MINT_TEST)); const is_containerized = is_containerized_deployment(); const account_options = is_containerized ? MINT_TEST.mint_account_params : MINT_TEST.nc_mint_account_params; await create_system_test_account(account_options); + + // Export AWS credentials for downstream SDK calls + const { access_key, secret_key } = await get_account_access_keys(account_options.name); + process.env.AWS_ACCESS_KEY_ID = access_key; + process.env.AWS_SECRET_ACCESS_KEY = secret_key; + console.info('MINT TEST SETUP DONE'); }src/test/system_tests/mint/mint_constants.js (2)
9-10: Ensure filesystem paths are created or made configurable.The hardcoded filesystem paths
/tmp/nsfs_root1/and/tmp/nsfs_root2/may not exist in all test environments and could cause test failures.Consider making these paths configurable via environment variables with defaults:
-const FS_ROOT_1 = '/tmp/nsfs_root1/'; -const FS_ROOT_2 = '/tmp/nsfs_root2/'; +const FS_ROOT_1 = process.env.NSFS_ROOT1 || '/tmp/nsfs_root1/'; +const FS_ROOT_2 = process.env.NSFS_ROOT2 || '/tmp/nsfs_root2/';Or ensure the directories are created during test setup by adding a setup hook in your test files.
31-33: Make UID/GID values configurable for better portability.The hardcoded UID and GID values (1000) may cause conflicts or permission issues in different environments. Consider making them configurable.
Apply this diff to use dynamic values:
- uid: 1000, - gid: 1000, + uid: Number(process.env.TEST_UID) || process.getuid(), + gid: Number(process.env.TEST_GID) || process.getgid(),This will use the current process's UID/GID by default while allowing override via environment variables.
🧹 Nitpick comments (13)
.github/workflows/mint-tests.yaml (3)
27-27: Remove trailing spaces.Line 27 has trailing spaces that should be removed.
- run: docker load --input /tmp/noobaa-tester.tar + run: docker load --input /tmp/noobaa-tester.tar
34-34: Remove trailing spaces.Line 34 has trailing spaces that should be removed.
- chmod 777 logs/mint-test-logs + chmod 777 logs/mint-test-logs
41-42: Remove extra blank lines.Remove the extra blank line at the end of the file.
- +src/test/system_tests/mint/configure_mint.js (1)
1-2: Remove redundant "use strict" directive.The "use strict" directive is redundant in ES modules as they are automatically in strict mode.
-/* Copyright (C) 2016 NooBaa */ -"use strict"; +/* Copyright (C) 2016 NooBaa */.github/workflows/mint-nc-tests.yaml (3)
1-1: Remove extra blank line at start.Remove the extra blank line at the beginning of the file.
- -name: Mint NC Tests +name: Mint NC Tests
35-35: Remove trailing spaces.Line 35 has trailing spaces that should be removed.
- chmod 777 logs/mint-nc-test-logs + chmod 777 logs/mint-nc-test-logs
41-42: Remove extra blank lines.Remove the extra blank line at the end of the file.
- +src/test/system_tests/mint/mint_constants.js (1)
1-2: Remove redundant "use strict" directive.The "use strict" directive is redundant in ES modules as they are automatically in strict mode.
-/* Copyright (C) 2022 NooBaa */ -'use strict'; +/* Copyright (C) 2022 NooBaa */docs/CI & Tests/Mint.md (5)
12-13: Lower-case “non-containerized” for consistencyThe term is normally written “non-containerized”. Consider revising.
34-35: Use dash (-) list markers to satisfy markdown-lint MD004Markdown-lint flags the asterisk style here. Switching to dashes keeps list style uniform with the rest of the repo.
-* [Mint Tests](../../.github/workflows/mint-tests.yaml) - Based on NooBaa Tester image, runs Mint on standard NooBaa (db configuration). -* [Mint NC Tests](../../.github/workflows/mint-nc-tests.yaml) - Based on NooBaa Tester image, runs Mint on non-containerized NooBaa (ConfigFS configuration). +- [Mint Tests](../../.github/workflows/mint-tests.yaml) - Based on NooBaa Tester image, runs Mint on standard NooBaa (db configuration). +- [Mint NC Tests](../../.github/workflows/mint-nc-tests.yaml) - Based on NooBaa Tester image, runs Mint on non-containerized NooBaa (ConfigFS configuration).
42-43: List-marker style also triggers MD004 hereSame lint warning as above; swap
*for-.-* `make test-mint` - Based on NooBaa Tester image, runs Mint on standard NooBaa (db configuration). -* `make test-nc-mint` - Based on NooBaa Tester image, runs Mint on non-containerized NooBaa (ConfigFS configuration). +- `make test-mint` - Based on NooBaa Tester image, runs Mint on standard NooBaa (db configuration). +- `make test-nc-mint` - Based on NooBaa Tester image, runs Mint on non-containerized NooBaa (ConfigFS configuration).
54-56: Fix ordered-list numberingStep “Run Mint” is numbered 2 again instead of 3.
-2. Tab 2 - Run Mint pointing to NooBaa endpoint - +3. Tab 3 - Run Mint pointing to NooBaa endpoint -
57-60: Add language identifier to fenced code blocks (MD040)Explicit language hints improve rendering & syntax highlighting.
-``` +```bash docker run -e SERVER_ENDPOINT=<noobaa-endpoint-address>:<noobaa-endpoint-http-port> -e ACCESS_KEY=<pre-existing-account-access-key> -e SECRET_KEY=<pre-existing-account-secret-key> -e ENABLE_HTTPS=0 minio/mint <sdk-or-tool-name>(and the same for the second block at lines 68-71) Also applies to: 68-71 </blockquote></details> </blockquote></details> <details> <summary>📜 Review details</summary> **Configuration used: .coderabbit.yaml** **Review profile: CHILL** **Plan: Pro** <details> <summary>📥 Commits</summary> Reviewing files that changed from the base of the PR and between c204edb937b5270f1d8cf476e738b52bbf91e0e2 and 1f2a8f636cde521a6f89038483235273389dd136. </details> <details> <summary>📒 Files selected for processing (13)</summary> * `.github/workflows/mint-nc-tests.yaml` (1 hunks) * `.github/workflows/mint-tests.yaml` (1 hunks) * `.github/workflows/run-pr-tests.yaml` (1 hunks) * `Makefile` (8 hunks) * `docs/CI & Tests/Mint.md` (1 hunks) * `src/endpoint/s3/s3_errors.js` (1 hunks) * `src/test/system_tests/external_tests_utils.js` (4 hunks) * `src/test/system_tests/mint/configure_mint.js` (1 hunks) * `src/test/system_tests/mint/mint_constants.js` (1 hunks) * `src/test/system_tests/mint/run_mint_on_test_container.sh` (1 hunks) * `src/test/system_tests/mint/run_nc_mint_on_test_container.sh` (1 hunks) * `src/test/system_tests/warp/configure_warp.js` (2 hunks) * `src/test/system_tests/warp/run_warp.js` (2 hunks) </details> <details> <summary>🚧 Files skipped from review as they are similar to previous changes (7)</summary> * src/endpoint/s3/s3_errors.js * src/test/system_tests/warp/configure_warp.js * src/test/system_tests/mint/run_mint_on_test_container.sh * src/test/system_tests/warp/run_warp.js * src/test/system_tests/mint/run_nc_mint_on_test_container.sh * Makefile * src/test/system_tests/external_tests_utils.js </details> <details> <summary>🧰 Additional context used</summary> <details> <summary>📓 Path-based instructions (1)</summary> <details> <summary>`src/test/**/*.*`: Ensure that the PR includes tests for the changes. </summary> > `src/test/**/*.*`: Ensure that the PR includes tests for the changes. > ⚙️ Source: CodeRabbit Configuration File List of files the instruction was applied to: - `src/test/system_tests/mint/configure_mint.js` - `src/test/system_tests/mint/mint_constants.js` </details> </details><details> <summary>🧠 Learnings (4)</summary> <details> <summary>📓 Common learnings</summary>Learnt from: nadavMiz
PR: #9099
File: .github/workflows/run-pr-tests.yaml:135-136
Timestamp: 2025-06-19T11:03:49.934Z
Learning: In the noobaa-core project, themake testercommand also callsmake noobaadue to makefile target dependencies, so runningmake testerwill build both the noobaa and tester images.</details> <details> <summary>.github/workflows/mint-tests.yaml (1)</summary>Learnt from: nadavMiz
PR: #9099
File: .github/workflows/run-pr-tests.yaml:52-70
Timestamp: 2025-06-19T10:01:46.933Z
Learning: In the noobaa-core repository, branch names don't contain forward slashes, so Docker tag sanitization is not needed in GitHub Actions workflows. Manual full builds and automated builds should use consistent tag naming to avoid mismatches.</details> <details> <summary>.github/workflows/run-pr-tests.yaml (2)</summary>Learnt from: nadavMiz
PR: #9099
File: .github/workflows/run-pr-tests.yaml:135-136
Timestamp: 2025-06-19T11:03:49.934Z
Learning: In the noobaa-core project, themake testercommand also callsmake noobaadue to makefile target dependencies, so runningmake testerwill build both the noobaa and tester images.Learnt from: nadavMiz
PR: #9099
File: .github/workflows/run-pr-tests.yaml:52-70
Timestamp: 2025-06-19T10:01:46.933Z
Learning: In the noobaa-core repository, branch names don't contain forward slashes, so Docker tag sanitization is not needed in GitHub Actions workflows. Manual full builds and automated builds should use consistent tag naming to avoid mismatches.</details> <details> <summary>docs/CI & Tests/Mint.md (1)</summary>Learnt from: nadavMiz
PR: #9099
File: .github/workflows/run-pr-tests.yaml:135-136
Timestamp: 2025-06-19T11:03:49.934Z
Learning: In the noobaa-core project, themake testercommand also callsmake noobaadue to makefile target dependencies, so runningmake testerwill build both the noobaa and tester images.</details> </details><details> <summary>🧬 Code Graph Analysis (2)</summary> <details> <summary>src/test/system_tests/mint/configure_mint.js (2)</summary><blockquote> <details> <summary>src/test/system_tests/external_tests_utils.js (1)</summary> * `account_options` (129-129) </details> <details> <summary>src/test/system_tests/mint/mint_constants.js (1)</summary> * `MINT_TEST` (14-42) </details> </blockquote></details> <details> <summary>src/test/system_tests/mint/mint_constants.js (2)</summary><blockquote> <details> <summary>src/test/system_tests/external_tests_utils.js (2)</summary> * `SensitiveString` (5-5) * `require` (6-6) </details> <details> <summary>src/test/system_tests/mint/configure_mint.js (2)</summary> * `require` (7-7) * `require` (8-8) </details> </blockquote></details> </details><details> <summary>🪛 YAMLlint (1.37.1)</summary> <details> <summary>.github/workflows/mint-nc-tests.yaml</summary> [warning] 1-1: too many blank lines (1 > 0) (empty-lines) --- [error] 35-35: trailing spaces (trailing-spaces) --- [warning] 41-41: too many blank lines (1 > 0) (empty-lines) </details> <details> <summary>.github/workflows/mint-tests.yaml</summary> [error] 27-27: trailing spaces (trailing-spaces) --- [error] 34-34: trailing spaces (trailing-spaces) --- [warning] 41-41: too many blank lines (2 > 0) (empty-lines) </details> <details> <summary>.github/workflows/run-pr-tests.yaml</summary> [warning] 45-45: wrong indentation: expected 8 but found 4 (indentation) --- [warning] 49-49: wrong indentation: expected 8 but found 4 (indentation) </details> </details> <details> <summary>🪛 LanguageTool</summary> <details> <summary>docs/CI & Tests/Mint.md</summary> [uncategorized] ~12-~12: A determiner appears to be missing. Consider inserting it. Context: ...inerized and Non Containerized flavors. Following are the SDKs/tools used in correctness ... (AI_EN_LECTOR_MISSING_DETERMINER) </details> </details> <details> <summary>🪛 markdownlint-cli2 (0.17.2)</summary> <details> <summary>docs/CI & Tests/Mint.md</summary> 34-34: Unordered list style Expected: dash; Actual: asterisk (MD004, ul-style) --- 35-35: Unordered list style Expected: dash; Actual: asterisk (MD004, ul-style) --- 42-42: Unordered list style Expected: dash; Actual: asterisk (MD004, ul-style) --- 43-43: Unordered list style Expected: dash; Actual: asterisk (MD004, ul-style) --- 57-57: Fenced code blocks should have a language specified (MD040, fenced-code-language) --- 68-68: Fenced code blocks should have a language specified (MD040, fenced-code-language) </details> </details> <details> <summary>🪛 Biome (1.9.4)</summary> <details> <summary>src/test/system_tests/mint/configure_mint.js</summary> [error] 1-2: Redundant use strict directive. The entire contents of JavaScript modules are automatically in strict mode, with no statement needed to initiate it. Safe fix: Remove the redundant use strict directive. (lint/suspicious/noRedundantUseStrict) </details> <details> <summary>src/test/system_tests/mint/mint_constants.js</summary> [error] 1-2: Redundant use strict directive. The entire contents of JavaScript modules are automatically in strict mode, with no statement needed to initiate it. Safe fix: Remove the redundant use strict directive. (lint/suspicious/noRedundantUseStrict) </details> </details> </details> <details> <summary>⏰ Context from checks skipped due to timeout of 90000ms (3)</summary> * GitHub Check: Build Noobaa Image * GitHub Check: run-package-lock-validation * GitHub Check: run-jest-unit-tests </details> <details> <summary>🔇 Additional comments (1)</summary><blockquote> <details> <summary>docs/CI & Tests/Mint.md (1)</summary> `80-88`: **Duplicate directory level in tree output** `mint-test-logs` appears twice in the path, which is confusing:├── mint-test-logs
│ ├── backingstore1.logVerify the actual folder hierarchy and remove the extra level if it’s a copy-paste artefact. </details> </blockquote></details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
| mint-tests: | ||
| needs: build-noobaa-image | ||
| uses: ./.github/workflows/mint-tests.yaml | ||
|
|
||
| mint-nc-tests: | ||
| needs: build-noobaa-image | ||
| uses: ./.github/workflows/mint-nc-tests.yaml |
There was a problem hiding this comment.
Fix YAML indentation for new jobs.
The new mint-tests and mint-nc-tests jobs have incorrect indentation. In GitHub Actions YAML, jobs should be indented with 8 spaces under the jobs: key.
Apply this diff to fix the indentation:
- mint-tests:
- needs: build-noobaa-image
- uses: ./.github/workflows/mint-tests.yaml
-
- mint-nc-tests:
- needs: build-noobaa-image
- uses: ./.github/workflows/mint-nc-tests.yaml
+ mint-tests:
+ needs: build-noobaa-image
+ uses: ./.github/workflows/mint-tests.yaml
+
+ mint-nc-tests:
+ needs: build-noobaa-image
+ uses: ./.github/workflows/mint-nc-tests.yamlCommittable suggestion skipped: line range outside the PR's diff.
🧰 Tools
🪛 YAMLlint (1.37.1)
[warning] 45-45: wrong indentation: expected 8 but found 4
(indentation)
[warning] 49-49: wrong indentation: expected 8 but found 4
(indentation)
🤖 Prompt for AI Agents
In .github/workflows/run-pr-tests.yaml around lines 44 to 50, the jobs
`mint-tests` and `mint-nc-tests` are incorrectly indented. To fix this, indent
both job definitions with 8 spaces under the `jobs:` key to comply with GitHub
Actions YAML syntax. Adjust the indentation of the entire job blocks
accordingly.
Signed-off-by: Romy <35330373+romayalon@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (5)
src/test/system_tests/mint/mint_constants.js (2)
9-10: Ensure/tmp/nsfs_root1/and/tmp/nsfs_root2/are created or made configurable.
31-33: Review hardcoded UID/GID values in system tests.src/test/system_tests/mint/configure_mint.js (1)
20-26: Export AWS credentials after account creation in mint tests..github/workflows/run-pr-tests.yaml (1)
44-50: Fix YAML indentation for new jobs..github/workflows/mint-nc-tests.yaml (1)
26-27: Fix artifact path for docker load command.
🧹 Nitpick comments (12)
src/test/system_tests/mint/mint_constants.js (1)
2-2: Remove redundant 'use strict' directive.JavaScript modules are automatically in strict mode, making this directive unnecessary.
-'use strict';src/test/system_tests/mint/configure_mint.js (1)
2-2: Remove redundant 'use strict' directive.JavaScript modules are automatically in strict mode, making this directive unnecessary.
-"use strict";.github/workflows/mint-tests.yaml (1)
27-27: Fix YAML formatting issues.Remove trailing spaces and excessive blank lines to comply with YAML linting rules.
- run: docker load --input /tmp/noobaa-tester.tar + run: docker load --input /tmp/noobaa-tester.tar - chmod 777 logs/mint-test-logs + chmod 777 logs/mint-test-logs -Also applies to: 34-34, 41-41
.github/workflows/mint-nc-tests.yaml (1)
1-1: Fix YAML formatting issues.Remove unnecessary blank lines and trailing spaces to comply with YAML linting rules.
- name: Mint NC Tests - chmod 777 logs/mint-nc-test-logs + chmod 777 logs/mint-nc-test-logs -Also applies to: 35-35, 41-41
docs/CI & Tests/Mint.md (8)
1-1: Standardize title capitalization and pluralityGitHub’s brand name is camel-cased and there is more than one action.
-# Mint Github Action, Tests and Tool +# Mint GitHub Actions, Tests and Tool
12-13: Tighten introduction grammarThe sentence is long, missing an article, and repeats “Containerized”. A crisper wording is easier to parse:
-[Mint](https://github.com/minio/mint) is a testing framework for S3-compatible object storage systems, NooBaa CI runs Mint as correctness/benchmarking and stress tests for the NooBaa system on both containerized and Non Containerized flavors. +[Mint](https://github.com/minio/mint) is a testing framework for S3-compatible object-storage systems. NooBaa CI runs Mint as correctness, benchmarking, and stress tests on both containerized and non-containerized deployments.
34-35: Consistent bullet style (markdownlint MD004)All other unordered lists use dashes. Switching from
*to-silences the linter and keeps the style uniform.-* [Mint Tests](../../.github/workflows/mint-tests.yaml) - Based on NooBaa Tester image, runs Mint on standard NooBaa (db configuration). -* [Mint NC Tests](../../.github/workflows/mint-nc-tests.yaml) - Based on NooBaa Tester image, runs Mint on non-containerized NooBaa (ConfigFS configuration). +- [Mint Tests](../../.github/workflows/mint-tests.yaml) - Based on NooBaa Tester image, runs Mint on standard NooBaa (DB configuration). +- [Mint NC Tests](../../.github/workflows/mint-nc-tests.yaml) - Based on NooBaa Tester image, runs Mint on non-containerized NooBaa (ConfigFS configuration).
42-43: Same bullet-style issue for Makefile targets-* `make test-mint` - Based on NooBaa Tester image, runs Mint on standard NooBaa (db configuration). -* `make test-nc-mint` - Based on NooBaa Tester image, runs Mint on non-containerized NooBaa (ConfigFS configuration). +- `make test-mint` – Runs Mint on standard NooBaa (DB deployment) using the tester image. +- `make test-nc-mint` – Runs Mint on non-containerized NooBaa (ConfigFS deployment) using the tester image.
54-56: Duplicate list index (“Tab 2”)Two consecutive steps are numbered “2”. Renumber or convert to bullets to avoid confusion.
-2. Tab 2 - Create a NooBaa account. -2. Tab 2 - Run Mint pointing to NooBaa endpoint - +2. Tab 2 – Create a NooBaa account. +3. Tab 3 – Run Mint pointing to the NooBaa endpoint:
57-60: Add language hint to fenced code block (markdownlint MD040)-``` +```bash docker run -e SERVER_ENDPOINT=<noobaa-endpoint-address>:<noobaa-endpoint-http-port> -e ACCESS_KEY=<pre-existing-account-access-key> -e SECRET_KEY=<pre-existing-account-secret-key> -e ENABLE_HTTPS=0 minio/mint <sdk-or-tool-name>--- `68-71`: **Same fenced-block language issue** ```diff -``` +```bash docker run -e SERVER_ENDPOINT=<noobaa-tester-container-id-or-name>:<noobaa-http-endpoint-port> -e ACCESS_KEY=<pre-existing-account-access-key> -e SECRET_KEY=<pre-existing-account-secret-key> -e ENABLE_HTTPS=0 --network noobaa-net minio/mint <sdk-or-tool-name>--- `94-99`: **Path inconsistency in debug-log example** Line 97 shows an absolute path (`/mint-nc-test-logs`) while the next line shows it under `logs/`. Stick to one path to prevent copy-paste errors. ```diff -noobaa-core % tree /mint-nc-test-logs -logs/mint-nc-test-logs +noobaa-core % tree logs/mint-nc-test-logs +logs/mint-nc-test-logs
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (13)
.github/workflows/mint-nc-tests.yaml(1 hunks).github/workflows/mint-tests.yaml(1 hunks).github/workflows/run-pr-tests.yaml(1 hunks)Makefile(9 hunks)docs/CI & Tests/Mint.md(1 hunks)src/endpoint/s3/s3_errors.js(1 hunks)src/test/system_tests/external_tests_utils.js(4 hunks)src/test/system_tests/mint/configure_mint.js(1 hunks)src/test/system_tests/mint/mint_constants.js(1 hunks)src/test/system_tests/mint/run_mint_on_test_container.sh(1 hunks)src/test/system_tests/mint/run_nc_mint_on_test_container.sh(1 hunks)src/test/system_tests/warp/configure_warp.js(2 hunks)src/test/system_tests/warp/run_warp.js(2 hunks)
✅ Files skipped from review due to trivial changes (1)
- src/test/system_tests/mint/run_mint_on_test_container.sh
🚧 Files skipped from review as they are similar to previous changes (6)
- src/endpoint/s3/s3_errors.js
- src/test/system_tests/warp/run_warp.js
- src/test/system_tests/mint/run_nc_mint_on_test_container.sh
- src/test/system_tests/warp/configure_warp.js
- Makefile
- src/test/system_tests/external_tests_utils.js
🧰 Additional context used
📓 Path-based instructions (1)
`src/test/**/*.*`: Ensure that the PR includes tests for the changes.
src/test/**/*.*: Ensure that the PR includes tests for the changes.
⚙️ Source: CodeRabbit Configuration File
List of files the instruction was applied to:
src/test/system_tests/mint/configure_mint.jssrc/test/system_tests/mint/mint_constants.js
🧠 Learnings (5)
📓 Common learnings
Learnt from: nadavMiz
PR: noobaa/noobaa-core#9099
File: .github/workflows/run-pr-tests.yaml:135-136
Timestamp: 2025-06-19T11:03:49.934Z
Learning: In the noobaa-core project, the `make tester` command also calls `make noobaa` due to makefile target dependencies, so running `make tester` will build both the noobaa and tester images.
.github/workflows/mint-nc-tests.yaml (2)
Learnt from: nadavMiz
PR: noobaa/noobaa-core#9099
File: .github/workflows/run-pr-tests.yaml:52-70
Timestamp: 2025-06-19T10:01:46.933Z
Learning: In the noobaa-core repository, branch names don't contain forward slashes, so Docker tag sanitization is not needed in GitHub Actions workflows. Manual full builds and automated builds should use consistent tag naming to avoid mismatches.
Learnt from: nadavMiz
PR: noobaa/noobaa-core#9099
File: .github/workflows/run-pr-tests.yaml:135-136
Timestamp: 2025-06-19T11:03:49.934Z
Learning: In the noobaa-core project, the `make tester` command also calls `make noobaa` due to makefile target dependencies, so running `make tester` will build both the noobaa and tester images.
.github/workflows/mint-tests.yaml (1)
Learnt from: nadavMiz
PR: noobaa/noobaa-core#9099
File: .github/workflows/run-pr-tests.yaml:52-70
Timestamp: 2025-06-19T10:01:46.933Z
Learning: In the noobaa-core repository, branch names don't contain forward slashes, so Docker tag sanitization is not needed in GitHub Actions workflows. Manual full builds and automated builds should use consistent tag naming to avoid mismatches.
.github/workflows/run-pr-tests.yaml (2)
Learnt from: nadavMiz
PR: noobaa/noobaa-core#9099
File: .github/workflows/run-pr-tests.yaml:135-136
Timestamp: 2025-06-19T11:03:49.934Z
Learning: In the noobaa-core project, the `make tester` command also calls `make noobaa` due to makefile target dependencies, so running `make tester` will build both the noobaa and tester images.
Learnt from: nadavMiz
PR: noobaa/noobaa-core#9099
File: .github/workflows/run-pr-tests.yaml:52-70
Timestamp: 2025-06-19T10:01:46.933Z
Learning: In the noobaa-core repository, branch names don't contain forward slashes, so Docker tag sanitization is not needed in GitHub Actions workflows. Manual full builds and automated builds should use consistent tag naming to avoid mismatches.
docs/CI & Tests/Mint.md (1)
Learnt from: nadavMiz
PR: noobaa/noobaa-core#9099
File: .github/workflows/run-pr-tests.yaml:135-136
Timestamp: 2025-06-19T11:03:49.934Z
Learning: In the noobaa-core project, the `make tester` command also calls `make noobaa` due to makefile target dependencies, so running `make tester` will build both the noobaa and tester images.
🧬 Code Graph Analysis (2)
src/test/system_tests/mint/configure_mint.js (2)
src/test/system_tests/external_tests_utils.js (1)
account_options(129-129)src/test/system_tests/mint/mint_constants.js (1)
MINT_TEST(14-42)
src/test/system_tests/mint/mint_constants.js (2)
src/test/system_tests/external_tests_utils.js (2)
SensitiveString(5-5)require(6-6)src/test/system_tests/mint/configure_mint.js (2)
require(7-7)require(8-8)
🪛 YAMLlint (1.37.1)
.github/workflows/mint-nc-tests.yaml
[warning] 1-1: too many blank lines (1 > 0)
(empty-lines)
[error] 35-35: trailing spaces
(trailing-spaces)
[warning] 41-41: too many blank lines (1 > 0)
(empty-lines)
.github/workflows/mint-tests.yaml
[error] 27-27: trailing spaces
(trailing-spaces)
[error] 34-34: trailing spaces
(trailing-spaces)
[warning] 41-41: too many blank lines (2 > 0)
(empty-lines)
.github/workflows/run-pr-tests.yaml
[warning] 45-45: wrong indentation: expected 8 but found 4
(indentation)
[warning] 49-49: wrong indentation: expected 8 but found 4
(indentation)
🪛 LanguageTool
docs/CI & Tests/Mint.md
[uncategorized] ~12-~12: A determiner appears to be missing. Consider inserting it.
Context: ...inerized and Non Containerized flavors. Following are the SDKs/tools used in correctness ...
(AI_EN_LECTOR_MISSING_DETERMINER)
🪛 markdownlint-cli2 (0.17.2)
docs/CI & Tests/Mint.md
34-34: Unordered list style
Expected: dash; Actual: asterisk
(MD004, ul-style)
35-35: Unordered list style
Expected: dash; Actual: asterisk
(MD004, ul-style)
42-42: Unordered list style
Expected: dash; Actual: asterisk
(MD004, ul-style)
43-43: Unordered list style
Expected: dash; Actual: asterisk
(MD004, ul-style)
57-57: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
68-68: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🪛 Biome (1.9.4)
src/test/system_tests/mint/configure_mint.js
[error] 1-2: Redundant use strict directive.
The entire contents of JavaScript modules are automatically in strict mode, with no statement needed to initiate it.
Safe fix: Remove the redundant use strict directive.
(lint/suspicious/noRedundantUseStrict)
src/test/system_tests/mint/mint_constants.js
[error] 1-2: Redundant use strict directive.
The entire contents of JavaScript modules are automatically in strict mode, with no statement needed to initiate it.
Safe fix: Remove the redundant use strict directive.
(lint/suspicious/noRedundantUseStrict)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: Build Noobaa Image
- GitHub Check: run-package-lock-validation
- GitHub Check: run-jest-unit-tests
🔇 Additional comments (5)
src/test/system_tests/mint/mint_constants.js (1)
14-42: Well-structured constants configuration.The MINT_TEST object is well-organized with clear separation between containerized and non-containerized account parameters. The use of SensitiveString for access keys is appropriate for security.
src/test/system_tests/mint/configure_mint.js (1)
10-18: Good error handling and exit code management.The main function properly handles errors with appropriate logging and exit codes, following best practices for CLI scripts.
.github/workflows/run-pr-tests.yaml (1)
44-50: LGTM: Proper job dependencies and workflow structure.The new mint test jobs follow the established pattern of depending on
build-noobaa-imageand referencing separate workflow files. This maintains consistency with the existing test structure..github/workflows/mint-tests.yaml (1)
1-12: Well-structured workflow with appropriate permissions.The workflow correctly defines permissions for artifact downloading and repository access. The timeout of 90 minutes is reasonable for system tests.
.github/workflows/mint-nc-tests.yaml (1)
2-12: Consistent workflow structure with mint-tests.yaml.The workflow follows the same pattern as the standard mint tests workflow, maintaining consistency in the CI pipeline. The permissions and timeout settings are appropriate.
| path: /tmp | ||
|
|
||
| - name: Load image | ||
| run: docker load --input /tmp/noobaa-tester.tar |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Verify artifact path is correct.
Based on past feedback, ensure the artifact path /tmp/noobaa-tester.tar is correct. If actions/download-artifact extracts to <path>/<artifact-name>/, the path should be /tmp/noobaa-tester/noobaa-tester.tar.
🏁 Script executed:
#!/bin/bash
# Description: Check how actions/download-artifact works with the specified path
# Search for other download-artifact usage patterns in the codebase
echo "=== Searching for download-artifact usage patterns ==="
rg -A 5 -B 2 "download-artifact" .github/workflows/
echo "=== Checking for docker load patterns ==="
rg -A 2 -B 2 "docker load" .github/workflows/Length of output: 14634
Update docker load path to include artifact directory
The actions/download-artifact step with path: /tmp will place the downloaded files under /tmp/noobaa-tester/, so the tarball ends up at /tmp/noobaa-tester/noobaa-tester.tar, not /tmp/noobaa-tester.tar.
Please update the “Load image” step in .github/workflows/mint-tests.yaml:
- File:
.github/workflows/mint-tests.yaml
Step: “Load image” (around line 26)
- run: docker load --input /tmp/noobaa-tester.tar
+ run: docker load --input /tmp/noobaa-tester/noobaa-tester.tar(You may also want to apply the same fix across the other workflows that use download-artifact name: noobaa-tester path: /tmp.)
🤖 Prompt for AI Agents
In .github/workflows/mint-tests.yaml at line 26, the docker load command uses an
incorrect path for the tarball file. Update the path from /tmp/noobaa-tester.tar
to /tmp/noobaa-tester/noobaa-tester.tar to match the artifact download location.
Also, check other workflows using the same artifact download path and apply the
same fix if needed.
Describe the Problem
Mint is a tool used for S3 correctentss and benchmarking tests that runs different S3 sdks, we would like to run it as part of NooBaa CI actions.
Explain the Changes
Issues: Fixed #xxx / Gap #xxx
Testing Instructions:
Summary by CodeRabbit
New Features
Bug Fixes
Refactor
Style