Skip to content

enable fchmodat_works test under android - #163823

Open
RalfJung wants to merge 1 commit into
rust-lang:mainfrom
RalfJung:android-test
Open

RalfJung wants to merge 1 commit into
rust-lang:mainfrom
RalfJung:android-test

Conversation

@RalfJung

@RalfJung RalfJung commented Oct 5, 2026 •

Copy link
Copy Markdown
Member

I missed this one in #163548. I don't know why we have what seems like duplicate tests for fs::set_permissions_nofollow, but more test coverage seems better so 🤷 .

@asder8215 -- all these tests got added in #158168 apparently; why the duplication?

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Oct 5, 2026
@rustbot rustbot added the T-libs Relevant to the library team, which will review and decide on the PR/issue. label Oct 5, 2026
@rustbot

rustbot commented Oct 5, 2026

Copy link
Copy Markdown
Collaborator

r? @ChrisDenton

rustbot has assigned @ChrisDenton.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

Why was this reviewer chosen?

The reviewer was selected based on:

  • Owners of files modified in this PR: @ChrisDenton, libs
  • @ChrisDenton, libs expanded to 13 candidates
  • Random selection from ChrisDenton, JohnTitor, Mark-Simulacrum, clarfonthey

@RalfJung

RalfJung commented Oct 5, 2026

Copy link
Copy Markdown
Member Author

@bors try jobs=android

@rust-bors

This comment has been minimized.

rust-bors Bot pushed a commit that referenced this pull request Oct 5, 2026
enable fchmodat_works test under android


try-job: *android*
check!(file.set_permissions(p));
}

#[cfg(not(target_os = "android"))]

@asder8215 asder8215 Oct 5, 2026 •

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.

Pretty sure I wrote this test earlier and templated this off the chmod_works test (if you look at lines 1699-1719 above). It does look redundant with the set_get_permissions_nofollows test on how it creates a regular file and calls set_permissions_nofollow on that. However, the only difference I can say between the two is that this one is intentionally trying to use set_permissions_nofollow on a file that doesn't exist.

Looking at it now, it's more like set_get_permissions_nofollows (which I based off set_get_unix_permissions() earlier and somehow it evolved into being a cross-platform test instead of unix only) is a duplicate of this test since this test encapsulates that test's behavior and more. The ErrorKind::Unsupported error on a regular file should only be reachable by hermit, vxworks and vexos, but I don't think CI tests for those, right?

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.

If you want to, you can remove the set_get_permissions_nofollows test to reduce bloat.

@rust-bors

rust-bors Bot commented Oct 5, 2026

Copy link
Copy Markdown
Contributor

☀️ Try build successful (CI)
Build commit: 25e3cd8 (25e3cd8c15ac176a1ec154b898e5e7f1102b9629)
Base parent: 602727f (602727f26878dcf9eb2999e2f58e5df1269ff47e)

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-libs Relevant to the library team, which will review and decide on the PR/issue.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants