Skip to content

Feat test n option - #1317

Merged
mvdan merged 1 commit into
mvdan:masterfrom
Mohamed-Moumni:feat-test-N-Option
May 31, 2026
Merged

mvdan merged 1 commit into
mvdan:masterfrom
Mohamed-Moumni:feat-test-N-Option

Conversation

@Mohamed-Moumni

Copy link
Copy Markdown
Contributor

Summary

  • Implement the -N file test operator (TsModif), which was previously unsupported and returned an error
  • Add interp/actime package with platform-specific GetAtime() functions (darwin, linux, windows) to retrieve file access time, as access time retrieval differs across platforms
  • Tested GetAtime() across all supported platforms, including Windows

Implemented #1316

@mvdan

mvdan commented Apr 9, 2026

Copy link
Copy Markdown
Owner

We can't just support the three main GOOSes; e.g. CI also covers js and plan9 because some users care about less widespread platforms. At least there should be a fallback file for other platforms that acts as a no-op by returning the mod time.

We need the type assertions to use the _, ok syntax and fall back to the mod time if the type doesn't match, because a custom handler may return an implementation of fs.FileInfo that doesn't contain this type in Sys.

Finally, name these files consistently with os_unix.go, i.e. os_linux.go, os_darwin.go, and so on.

@Mohamed-Moumni

Copy link
Copy Markdown
Contributor Author

Thanks for the review. I added os_other.go for unhandled platforms, using mod time as a fallback value, and fixed the type assertions to use the _, ok syntax with a mod time fallback.

Changes Made

  • actime_*.go → os_*.go
  • Added the type assertions with mod time as a fallback return

@mvdan

mvdan commented Apr 13, 2026

Copy link
Copy Markdown
Owner

you're exposing public APIs. make these internal APIs in the interp package

@Mohamed-Moumni

Copy link
Copy Markdown
Contributor Author

I internalized the actime logic, so it's only accessible via the interp package.

@mvdan

mvdan commented Apr 13, 2026

Copy link
Copy Markdown
Owner

I meant in the actual interp package. I'll tweak this myself and merge it later.

@Mohamed-Moumni

Copy link
Copy Markdown
Contributor Author

Sorry for the confusion, I misunderstood the req, It's fixed now.

@mvdan

mvdan commented Apr 16, 2026

Copy link
Copy Markdown
Owner

The test fails on Windows. Not sure if the test is too flakey with the sleep, or if it's just incorrect on Windows. I'll have to investigate.

@mvdan

mvdan commented May 31, 2026

Copy link
Copy Markdown
Owner

It looks like the real windows does not really support access times by default. I'm making the tests unix-only.

@mvdan
mvdan force-pushed the feat-test-N-Option branch from b2f429d to 0210079 Compare May 31, 2026 21:43
Add platform-specific access time retrieval via the new actime package
(darwin, linux, windows) and use it to check if a file was modified
since it was last read (mtime > atime).

Co-authored-by: Daniel Martí <mvdan@mvdan.cc>
@mvdan
mvdan force-pushed the feat-test-N-Option branch from 0210079 to 1d9358d Compare May 31, 2026 21:48
@mvdan
mvdan merged commit ed87bb0 into mvdan:master May 31, 2026
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants