Skip to content

Conversation

aayushchouhan09
Copy link
Member

@aayushchouhan09 aayushchouhan09 commented Jul 22, 2025

Describe the Problem

After updating the ceph commit hash, s3 tests were failing for both NSFS and non-NSFS configurations. The failures were caused by missing required configuration fields that the updated ceph test suite expects.

Explain the Changes

  1. Added the missing fields in the config file with dummy values currently, we will update it in future when we support the functionality.
  2. Updated the commit hash to latest.
  3. Moved new tests to the blacklist.
  4. Two tests (regressoin) are failing, moved to pending list and updated the doc with the root cause.

s3tests_boto3/functional/test_headers.py::test_object_create_bad_md5_bad
s3tests_boto3/functional/test_headers.py::test_object_create_bad_md5_none

nsfs-ceph-s3 output:

Jul-22 16:43:03.672 [test_ceph_s3/121] [LOG] CONSOLE:: Finished Running Ceph S3 Tests
Jul-22 16:43:03.672 [test_ceph_s3/121] [INFO] CONSOLE:: CEPH TEST SUMMARY: Suite contains 1005, ran 369 tests, Passed: 230, Skipped: 139, Failed: 0
Jul-22 16:43:03.672 [test_ceph_s3/121] [WARN] CONSOLE:: CEPH TEST SKIPPED TESTS SUMMARY: 139 skipped tests

ceph-s3 output:

Jul-31 14:34:23.880 [test_ceph_s3/182] [LOG] CONSOLE:: Finished Running Ceph S3 Tests
Jul-31 14:34:23.880 [test_ceph_s3/182] [INFO] CONSOLE:: CEPH TEST SUMMARY: Suite contains 1008, ran 425 tests, Passed: 286, Skipped: 139, Failed: 0
Jul-31 14:34:23.880 [test_ceph_s3/182] [WARN] CONSOLE:: CEPH TEST SKIPPED TESTS SUMMARY: 139 skipped tests

GAP: There are more ceph-tests (regression) which are failing and moved to pending list, but not sure about the root cause need to investigate.

3tests_boto3/functional/test_headers.py::test_object_create_bad_md5_bad
s3tests_boto3/functional/test_headers.py::test_object_create_bad_md5_none
s3tests_boto3/functional/test_headers.py::test_object_create_bad_expect_mismatch
s3tests_boto3/functional/test_headers.py::test_object_create_bad_expect_empty
s3tests_boto3/functional/test_headers.py::test_object_create_bad_expect_none

Issues: Fixed #xxx / Gap #xxx

  1. Fixed: https://issues.redhat.com/browse/MCGI-254
  2. Fixed Gap: CI | Update Ceph S3 Tests Days (Temporary Solution) #8392

Testing Instructions:

  1. make test-cephs3 and make test-nsfs-cephs3
  • Doc added/updated

Summary by CodeRabbit

Summary by CodeRabbit

  • New Features

    • Expanded configuration options for Ceph S3 tests, including new IAM-related settings and structured sections for IAM accounts and credentials.
  • Chores

    • Updated the Ceph S3 tests repository to a newer version with a reduced commit age threshold.
    • Improved configuration setup by standardizing key insertion through placeholder substitution.
    • Enhanced test selection filtering to target specific test identifiers.
    • Added numerous new test cases to pending test lists covering extensive S3 functionality.
    • Extended test blacklists to exclude additional S3 and STS tests due to known incompatibilities.
  • Documentation

    • Added entries to pending tests documentation highlighting known AWS v2 signature issues due to botocore updates.

@aayushchouhan09 aayushchouhan09 requested review from a team, alphaprinz, nadavMiz and romayalon and removed request for a team July 22, 2025 11:36
@coderabbitai
Copy link

coderabbitai bot commented Jul 22, 2025

Walkthrough

The changes update the Ceph S3 test configuration to include new IAM-related sections and keys, adjust the setup scripts to use placeholder substitution for access/secret keys via sed on macOS and other platforms, modify the test listing command to filter more precisely, expand pending and blacklist test lists with many new entries, update the test deployment script to use a newer commit of the Ceph S3 tests repository, and add documentation entries for known test failures related to botocore updates. No core logic or control flow is altered.

Changes

File(s) Change Summary
Ceph S3 test configuration
src/test/external_tests/ceph_s3_tests/test_ceph_s3_config.conf
Added new IAM-related sections and keys to support IAM and sts-related tests.
Setup scripts for config substitution
src/test/external_tests/ceph_s3_tests/test_ceph_s3_config_setup.js, src/test/external_tests/ceph_s3_tests/test_ceph_nsfs_s3_config_setup.js
Replaced direct key appending with placeholder substitution for access/secret keys using sed on macOS and others.
Test listing command
src/test/external_tests/ceph_s3_tests/test_ceph_s3.js
Modified test listing command to filter lines containing "::test" instead of any "test".
Deployment script
src/test/external_tests/ceph_s3_tests/test_ceph_s3_deploy.sh
Updated the Ceph S3 tests repository commit hash to a newer version and reduced max allowed commit age.
Pending test lists
src/test/external_tests/ceph_s3_tests/s3-tests-lists/nsfs_s3_tests_pending_list.txt, src/test/external_tests/ceph_s3_tests/s3-tests-lists/s3_tests_pending_list.txt
Added new test entries to pending test lists.
Blacklist test lists
src/test/external_tests/ceph_s3_tests/s3-tests-lists/nsfs_s3_tests_black_list.txt, src/test/external_tests/ceph_s3_tests/s3-tests-lists/s3_tests_black_list.txt
Extended blacklists with many new test entries covering various S3 and STS features.
Documentation
docs/dev_guide/ceph_s3_tests/ceph_s3_tests_pending_list_status.md
Added documentation entries for two new pending tests related to botocore update breaking AWS v2 signature handling.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~40 minutes

Possibly related PRs

Suggested labels

size/XS, Enable-Auto-Rebase

Suggested reviewers

  • nadavMiz

Note

⚡️ Unit Test Generation is now available in beta!

Learn more here, or try it out under "Finishing Touches" below.


📜 Recent review details

Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between e47cc28 and d02507b.

📒 Files selected for processing (10)
  • docs/dev_guide/ceph_s3_tests/ceph_s3_tests_pending_list_status.md (1 hunks)
  • src/test/external_tests/ceph_s3_tests/s3-tests-lists/nsfs_s3_tests_black_list.txt (1 hunks)
  • src/test/external_tests/ceph_s3_tests/s3-tests-lists/nsfs_s3_tests_pending_list.txt (1 hunks)
  • src/test/external_tests/ceph_s3_tests/s3-tests-lists/s3_tests_black_list.txt (1 hunks)
  • src/test/external_tests/ceph_s3_tests/s3-tests-lists/s3_tests_pending_list.txt (1 hunks)
  • src/test/external_tests/ceph_s3_tests/test_ceph_nsfs_s3_config_setup.js (1 hunks)
  • src/test/external_tests/ceph_s3_tests/test_ceph_s3.js (1 hunks)
  • src/test/external_tests/ceph_s3_tests/test_ceph_s3_config.conf (2 hunks)
  • src/test/external_tests/ceph_s3_tests/test_ceph_s3_config_setup.js (1 hunks)
  • src/test/external_tests/ceph_s3_tests/test_ceph_s3_deploy.sh (2 hunks)
✅ Files skipped from review due to trivial changes (2)
  • src/test/external_tests/ceph_s3_tests/s3-tests-lists/nsfs_s3_tests_pending_list.txt
  • src/test/external_tests/ceph_s3_tests/test_ceph_s3.js
🚧 Files skipped from review as they are similar to previous changes (7)
  • src/test/external_tests/ceph_s3_tests/test_ceph_s3_deploy.sh
  • src/test/external_tests/ceph_s3_tests/test_ceph_nsfs_s3_config_setup.js
  • src/test/external_tests/ceph_s3_tests/test_ceph_s3_config_setup.js
  • docs/dev_guide/ceph_s3_tests/ceph_s3_tests_pending_list_status.md
  • src/test/external_tests/ceph_s3_tests/s3-tests-lists/s3_tests_pending_list.txt
  • src/test/external_tests/ceph_s3_tests/s3-tests-lists/s3_tests_black_list.txt
  • src/test/external_tests/ceph_s3_tests/test_ceph_s3_config.conf
🧰 Additional context used
📓 Path-based instructions (1)
src/test/**/*.*

⚙️ CodeRabbit Configuration File

src/test/**/*.*: Ensure that the PR includes tests for the changes.

Files:

  • src/test/external_tests/ceph_s3_tests/s3-tests-lists/nsfs_s3_tests_black_list.txt
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
  • GitHub Check: Build Noobaa Image
  • GitHub Check: run-jest-unit-tests
  • GitHub Check: run-package-lock-validation
✨ Finishing Touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Explain this complex logic.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai explain this code block.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and explain its main purpose.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

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 using PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR.
  • @coderabbitai generate sequence diagram to generate a sequence diagram of the changes in this PR.
  • @coderabbitai generate unit tests to generate unit tests for this PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

📜 Review details

Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 24e11b7 and eb679ea.

📒 Files selected for processing (3)
  • src/test/external_tests/ceph_s3_tests/test_ceph_s3_config.conf (2 hunks)
  • src/test/external_tests/ceph_s3_tests/test_ceph_s3_config_setup.js (1 hunks)
  • src/test/external_tests/ceph_s3_tests/test_ceph_s3_deploy.sh (1 hunks)
📓 Path-based instructions (1)
src/test/**/*.*

⚙️ CodeRabbit Configuration File

src/test/**/*.*: Ensure that the PR includes tests for the changes.

Files:

  • src/test/external_tests/ceph_s3_tests/test_ceph_s3_deploy.sh
  • src/test/external_tests/ceph_s3_tests/test_ceph_s3_config_setup.js
  • src/test/external_tests/ceph_s3_tests/test_ceph_s3_config.conf
🧰 Additional context used
📓 Path-based instructions (1)
src/test/**/*.*

⚙️ CodeRabbit Configuration File

src/test/**/*.*: Ensure that the PR includes tests for the changes.

Files:

  • src/test/external_tests/ceph_s3_tests/test_ceph_s3_deploy.sh
  • src/test/external_tests/ceph_s3_tests/test_ceph_s3_config_setup.js
  • src/test/external_tests/ceph_s3_tests/test_ceph_s3_config.conf
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
  • GitHub Check: Build Noobaa Image
  • GitHub Check: run-jest-unit-tests
🔇 Additional comments (4)
src/test/external_tests/ceph_s3_tests/test_ceph_s3_deploy.sh (1)

20-20: Validated new CEPH_TESTS_VERSION hash

  • The commit aafacab2017bec1dd1ce3159105455bb7e9447b1 exists in the ceph/s3-tests repository on master.
  • It is only 4 days old, well within the 600-day limit.

No further changes required.

src/test/external_tests/ceph_s3_tests/test_ceph_s3_config_setup.js (1)

95-96: Good improvement in credential injection consistency.

The change from appending keys to replacing placeholders using sed improves consistency and aligns well with the updated config file structure. The OS-specific handling for macOS is correctly implemented.

Also applies to: 101-102

src/test/external_tests/ceph_s3_tests/test_ceph_s3_config.conf (2)

22-28: LGTM: IAM naming configuration added.

The new IAM naming prefixes support the enhanced IAM functionality mentioned in the PR objectives.


62-63: LGTM: Tenant name configuration added.

The tenant field addition supports enhanced tenant testing capabilities.

@aayushchouhan09 aayushchouhan09 marked this pull request as draft July 22, 2025 12:40
@pull-request-size pull-request-size bot added size/L and removed size/M labels Jul 22, 2025
@aayushchouhan09 aayushchouhan09 marked this pull request as ready for review July 23, 2025 04:56
@aayushchouhan09 aayushchouhan09 force-pushed the ceph-fix branch 7 times, most recently from e59a8e5 to 2c0d694 Compare July 28, 2025 10:46
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Nitpick comments (2)
src/test/external_tests/ceph_s3_tests/s3-tests-lists/nsfs_s3_tests_black_list.txt (2)

435-471: Collapse the bucket-logging suite with a wildcard entry

Lines 435-471 list 37 separate test_bucket_logging_* cases.
If the blacklist parser accepts pytest-style globs, replace them with one line:

s3tests_boto3/functional/test_s3.py::test_bucket_logging*

This keeps the file short and automatically covers new variants that may appear.


426-434: Wildcard the checksum tests to avoid future omissions

The checksum-related tests (426-434) could similarly be collapsed to:

s3tests_boto3/functional/test_s3.py::test_*checksum*

Should more checksum variants be added upstream, they will be blacklisted automatically.

📜 Review details

Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 2c03fd4 and 2c0d694.

📒 Files selected for processing (10)
  • docs/dev_guide/ceph_s3_tests/ceph_s3_tests_pending_list_status.md (1 hunks)
  • src/test/external_tests/ceph_s3_tests/s3-tests-lists/nsfs_s3_tests_black_list.txt (1 hunks)
  • src/test/external_tests/ceph_s3_tests/s3-tests-lists/nsfs_s3_tests_pending_list.txt (1 hunks)
  • src/test/external_tests/ceph_s3_tests/s3-tests-lists/s3_tests_black_list.txt (1 hunks)
  • src/test/external_tests/ceph_s3_tests/s3-tests-lists/s3_tests_pending_list.txt (1 hunks)
  • src/test/external_tests/ceph_s3_tests/test_ceph_nsfs_s3_config_setup.js (1 hunks)
  • src/test/external_tests/ceph_s3_tests/test_ceph_s3.js (1 hunks)
  • src/test/external_tests/ceph_s3_tests/test_ceph_s3_config.conf (2 hunks)
  • src/test/external_tests/ceph_s3_tests/test_ceph_s3_config_setup.js (1 hunks)
  • src/test/external_tests/ceph_s3_tests/test_ceph_s3_deploy.sh (1 hunks)
✅ Files skipped from review due to trivial changes (2)
  • src/test/external_tests/ceph_s3_tests/test_ceph_s3.js
  • src/test/external_tests/ceph_s3_tests/s3-tests-lists/s3_tests_black_list.txt
🚧 Files skipped from review as they are similar to previous changes (7)
  • docs/dev_guide/ceph_s3_tests/ceph_s3_tests_pending_list_status.md
  • src/test/external_tests/ceph_s3_tests/test_ceph_s3_deploy.sh
  • src/test/external_tests/ceph_s3_tests/test_ceph_nsfs_s3_config_setup.js
  • src/test/external_tests/ceph_s3_tests/test_ceph_s3_config_setup.js
  • src/test/external_tests/ceph_s3_tests/s3-tests-lists/s3_tests_pending_list.txt
  • src/test/external_tests/ceph_s3_tests/s3-tests-lists/nsfs_s3_tests_pending_list.txt
  • src/test/external_tests/ceph_s3_tests/test_ceph_s3_config.conf
🧰 Additional context used
📓 Path-based instructions (1)
src/test/**/*.*

⚙️ CodeRabbit Configuration File

src/test/**/*.*: Ensure that the PR includes tests for the changes.

Files:

  • src/test/external_tests/ceph_s3_tests/s3-tests-lists/nsfs_s3_tests_black_list.txt

Copy link
Contributor

@alphaprinz alphaprinz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's quite a lot of black listed tests.
Is there a recurring pattern in their failure?

Also, I see that tests still failing.
Annoyingly, I am unable to get the log of the run, it's too long and Chrome hangs.

@aayushchouhan09
Copy link
Member Author

@alphaprinz

  1. The tests which are put in the blacklist are the new tests introduced in the ceph.
  2. There were two more tests (regression) which are failing the CI, so I moved them them to pending list.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

♻️ Duplicate comments (1)
src/test/external_tests/ceph_s3_tests/s3-tests-lists/nsfs_s3_tests_black_list.txt (1)

47-50: Duplicate test entries – issue already raised earlier

The four TestFileValidator / TestFiles items at Lines 47-50 already exist (see Lines 283-286).
They should appear only once in the blacklist.

Because this exact problem was pointed out in the previous review and still persists, I’m tagging this as a duplicate remark.

📜 Review details

Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 063231c and 869ca67.

📒 Files selected for processing (10)
  • docs/dev_guide/ceph_s3_tests/ceph_s3_tests_pending_list_status.md (1 hunks)
  • src/test/external_tests/ceph_s3_tests/s3-tests-lists/nsfs_s3_tests_black_list.txt (1 hunks)
  • src/test/external_tests/ceph_s3_tests/s3-tests-lists/nsfs_s3_tests_pending_list.txt (1 hunks)
  • src/test/external_tests/ceph_s3_tests/s3-tests-lists/s3_tests_black_list.txt (1 hunks)
  • src/test/external_tests/ceph_s3_tests/s3-tests-lists/s3_tests_pending_list.txt (1 hunks)
  • src/test/external_tests/ceph_s3_tests/test_ceph_nsfs_s3_config_setup.js (1 hunks)
  • src/test/external_tests/ceph_s3_tests/test_ceph_s3.js (1 hunks)
  • src/test/external_tests/ceph_s3_tests/test_ceph_s3_config.conf (2 hunks)
  • src/test/external_tests/ceph_s3_tests/test_ceph_s3_config_setup.js (1 hunks)
  • src/test/external_tests/ceph_s3_tests/test_ceph_s3_deploy.sh (2 hunks)
✅ Files skipped from review due to trivial changes (1)
  • src/test/external_tests/ceph_s3_tests/s3-tests-lists/nsfs_s3_tests_pending_list.txt
🚧 Files skipped from review as they are similar to previous changes (7)
  • src/test/external_tests/ceph_s3_tests/test_ceph_s3.js
  • docs/dev_guide/ceph_s3_tests/ceph_s3_tests_pending_list_status.md
  • src/test/external_tests/ceph_s3_tests/s3-tests-lists/s3_tests_pending_list.txt
  • src/test/external_tests/ceph_s3_tests/test_ceph_nsfs_s3_config_setup.js
  • src/test/external_tests/ceph_s3_tests/test_ceph_s3_deploy.sh
  • src/test/external_tests/ceph_s3_tests/test_ceph_s3_config_setup.js
  • src/test/external_tests/ceph_s3_tests/test_ceph_s3_config.conf
🧰 Additional context used
📓 Path-based instructions (1)
src/test/**/*.*

⚙️ CodeRabbit Configuration File

src/test/**/*.*: Ensure that the PR includes tests for the changes.

Files:

  • src/test/external_tests/ceph_s3_tests/s3-tests-lists/nsfs_s3_tests_black_list.txt
  • src/test/external_tests/ceph_s3_tests/s3-tests-lists/s3_tests_black_list.txt
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
  • GitHub Check: Build Noobaa Image
  • GitHub Check: run-jest-unit-tests
  • GitHub Check: run-package-lock-validation
🔇 Additional comments (1)
src/test/external_tests/ceph_s3_tests/s3-tests-lists/s3_tests_black_list.txt (1)

352-437: Sorted and Deduplicated s3_tests_black_list.txt

  • The file src/test/external_tests/ceph_s3_tests/s3-tests-lists/s3_tests_black_list.txt has been run through sort -u, integrating the appended block into the main body.
  • Verified with a duplicate check script: no duplicate entries remain.

File is now alphabetically sorted, unique, and in a single canonical form.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🔭 Outside diff range comments (1)
src/test/external_tests/ceph_s3_tests/s3-tests-lists/s3_tests_black_list.txt (1)

1-437: De-duplicate s3_tests_black_list.txt entries

Found the following duplicate lines in
src/test/external_tests/ceph_s3_tests/s3-tests-lists/s3_tests_black_list.txt:

• test_realistic/TestFileValidator.py::test_new_file_is_valid
• test_realistic/TestFileValidator.py::test_new_file_is_valid_on_several_calls
• test_realistic/TestFileValidator.py::test_new_file_is_valid_when_size_is_1
• test_realistic/TestFiles/test_random_file_valid

These duplicates:

  • Add noise to diffs
  • Risk diverging comments/reasons if metadata is added later
  • Indicate that list maintenance is error-prone

Please remove the repeated lines and add a CI check to prevent future duplicates. For example:

#!/bin/bash
set -e
dups=$(sort src/test/external_tests/ceph_s3_tests/s3-tests-lists/s3_tests_black_list.txt | uniq -d)
if [ -n "$dups" ]; then
  echo "❌ Duplicate entries found:" >&2
  echo "$dups" >&2
  exit 1
fi
echo "✅ No duplicates."

[file to fix]
• src/test/external_tests/ceph_s3_tests/s3-tests-lists/s3_tests_black_list.txt

📜 Review details

Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 869ca67 and b5dcd74.

📒 Files selected for processing (10)
  • docs/dev_guide/ceph_s3_tests/ceph_s3_tests_pending_list_status.md (1 hunks)
  • src/test/external_tests/ceph_s3_tests/s3-tests-lists/nsfs_s3_tests_black_list.txt (1 hunks)
  • src/test/external_tests/ceph_s3_tests/s3-tests-lists/nsfs_s3_tests_pending_list.txt (1 hunks)
  • src/test/external_tests/ceph_s3_tests/s3-tests-lists/s3_tests_black_list.txt (1 hunks)
  • src/test/external_tests/ceph_s3_tests/s3-tests-lists/s3_tests_pending_list.txt (1 hunks)
  • src/test/external_tests/ceph_s3_tests/test_ceph_nsfs_s3_config_setup.js (1 hunks)
  • src/test/external_tests/ceph_s3_tests/test_ceph_s3.js (1 hunks)
  • src/test/external_tests/ceph_s3_tests/test_ceph_s3_config.conf (2 hunks)
  • src/test/external_tests/ceph_s3_tests/test_ceph_s3_config_setup.js (1 hunks)
  • src/test/external_tests/ceph_s3_tests/test_ceph_s3_deploy.sh (2 hunks)
✅ Files skipped from review due to trivial changes (1)
  • src/test/external_tests/ceph_s3_tests/s3-tests-lists/nsfs_s3_tests_pending_list.txt
🚧 Files skipped from review as they are similar to previous changes (8)
  • src/test/external_tests/ceph_s3_tests/test_ceph_s3.js
  • src/test/external_tests/ceph_s3_tests/s3-tests-lists/s3_tests_pending_list.txt
  • src/test/external_tests/ceph_s3_tests/test_ceph_s3_deploy.sh
  • src/test/external_tests/ceph_s3_tests/test_ceph_s3_config_setup.js
  • src/test/external_tests/ceph_s3_tests/s3-tests-lists/nsfs_s3_tests_black_list.txt
  • docs/dev_guide/ceph_s3_tests/ceph_s3_tests_pending_list_status.md
  • src/test/external_tests/ceph_s3_tests/test_ceph_nsfs_s3_config_setup.js
  • src/test/external_tests/ceph_s3_tests/test_ceph_s3_config.conf
🧰 Additional context used
📓 Path-based instructions (1)
src/test/**/*.*

⚙️ CodeRabbit Configuration File

src/test/**/*.*: Ensure that the PR includes tests for the changes.

Files:

  • src/test/external_tests/ceph_s3_tests/s3-tests-lists/s3_tests_black_list.txt
🔇 Additional comments (1)
src/test/external_tests/ceph_s3_tests/s3-tests-lists/s3_tests_black_list.txt (1)

365-371: Overlap with pending lists – ensure tests are not excluded twice

Tests such as test_bucket_policy_multipart and test_bucket_policy_tenanted_bucket were just black-listed, but they were also moved to pending in the same PR according to the description.
A test should live in exactly one list; otherwise it is unclear whether it is being skipped for “known failure” or “under investigation”.
Please cross-check s3_tests_pending_list.txt and nsfs_s3_tests_pending_list.txt for duplicates before merging.

@aayushchouhan09 aayushchouhan09 merged commit f559d9b into noobaa:master Jul 31, 2025
28 of 31 checks passed
@aayushchouhan09 aayushchouhan09 deleted the ceph-fix branch July 31, 2025 14:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants