Skip to content

GH-50665: [Python] Assert s3fs selector result count - #50389

Draft
anxkhn wants to merge 1 commit into
apache:mainfrom
anxkhn:test/fs-selector-s3fs-count-assert
Draft

GH-50665: [Python] Assert s3fs selector result count#50389
anxkhn wants to merge 1 commit into
apache:mainfrom
anxkhn:test/fs-selector-s3fs-count-assert

Conversation

@anxkhn

@anxkhn anxkhn commented Jul 6, 2026

Copy link
Copy Markdown

Rationale for this change

The fsspec S3 branch of test_get_file_info_with_selector evaluates len(infos) == 4 without asserting it, so an incorrect recursive listing count can pass the test.

Closes #50665.

What changes are included in this PR?

Add the missing assert to the existing comparison. No production code or expected value changes.

Are these changes tested?

The Python CI matrix passed on the original one-line change. The rebased diff passes git diff --check.

Are there any user-facing changes?

No.

This change was made with help from an AI coding assistant. I reviewed the one-line diff and verified it against the surrounding assertions.

Copilot AI review requested due to automatic review settings July 6, 2026 12:22
@anxkhn
anxkhn requested review from AlenkaF, raulcd and rok as code owners July 6, 2026 12:22

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Fixes a no-op comparison in the Python filesystem test suite so that the s3fs-backed fsspec S3 branch actually asserts the expected recursive selector result count, aligning it with the other branches in test_get_file_info_with_selector.

Changes:

  • Replace a discarded boolean expression with an assert in the s3fs recursive-selector branch.
  • Ensure the recursive listing count is verified for fsspec S3 (s3fs) filesystems.

@raulcd raulcd left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This is the same type of AI slop PRs than:

@anxkhn please read the previous comment on the previous PR and fix it or stop posting this.

@raulcd
raulcd marked this pull request as draft July 6, 2026 13:07
@github-actions github-actions Bot added awaiting changes Awaiting changes and removed awaiting review Awaiting review labels Jul 6, 2026
Assert the recursive selector count for the fsspec S3 branch instead of discarding the comparison result. This aligns the branch with the existing count checks for other filesystems.

Signed-off-by: Anas Khan <83116240+anxkhn@users.noreply.github.com>
@anxkhn
anxkhn force-pushed the test/fs-selector-s3fs-count-assert branch from fdeebed to a7dd526 Compare July 27, 2026 17:51
@github-actions github-actions Bot added awaiting change review Awaiting change review and removed awaiting changes Awaiting changes labels Jul 27, 2026
@anxkhn anxkhn changed the title MINOR: [Python] Assert selector file count on the s3fs branch in test_get_file_info_with_selector GH-50665: [Python] Assert s3fs selector result count Jul 27, 2026
@anxkhn

anxkhn commented Jul 27, 2026

Copy link
Copy Markdown
Author

Pushed a revision addressing the latest review/CI feedback.

@github-actions

Copy link
Copy Markdown

⚠️ GitHub issue #50665 has been automatically assigned in GitHub to PR creator.

@anxkhn

anxkhn commented Jul 27, 2026

Copy link
Copy Markdown
Author

opened #50665 because this executable test change is not eligible for the documentation-only minor exception, then updated the title, body, and signed-off commit. the branch is rebased onto current main. @raulcd could you please re-review?

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.

[Python] s3fs selector count is not asserted

3 participants