Skip to content

Conversation

@reubeno
Copy link
Contributor

@reubeno reubeno commented Mar 16, 2025

Taking inspiration from #23, expands PR/CI testing via matrix:

  • Adds build-only validation on 1.65.0 (see below)
  • Adds test validation on 1.71.0, stable, and nightly (x86_64/Linux)
  • Adds test validation on arm64 Linux (stable)
  • Adds unit-test-only validation on macOS (stable)

Also adds 2 basic integration tests for all_users and all_groups.


Notes:

  • Regarding min Rust versions, I used cargo msrv to empirically compute the effective MSRV of this crate as 1.65.0. I then discovered that the dev dependency on env_logger for testing raises the minimum for testing as being 1.71.0. I ended up adding both to the matrix, disabling test execution on the older version.

  • On macOS, libnss_wrapper doesn't appear available so I've only enabled unit tests there until an alternate strategy could be adopted.

Potential future work:

  • Formally document MSRV via metadata in Cargo.toml.
  • At least validate build for alternate targets (e.g., BSD families) using cross or cargo --target.
  • Explore use of vmactions GitHub actions for running build-and-test on alternate OSes using VMs hosted in PR/CI workflows.

@reubeno
Copy link
Contributor Author

reubeno commented Apr 3, 2025

Hi @gierens -- I've posted this PR as an attempt to contribute toward issue #23. This would be my first contribution to this project, and I'd be grateful for any feedback you'd have.

Thanks!

Copy link
Member

@gierens gierens left a comment

Choose a reason for hiding this comment

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

Hi @reubeno,

Sorry for the late reply, I've been quite busy the last couple of weeks. Anyway, thanks for this clean and well documented PR. LGTM! One minor thing, since the added tests are an unrelated change, I split this into two commits.

Regarding future work, yeah, documenting the MSRV is a good idea. As for testing on other targets, yeah, we are using vmactions on eza, too.

@gierens gierens merged commit 1366bbd into rustadopt:main Apr 4, 2025
10 checks passed
@reubeno reubeno deleted the testing branch April 4, 2025 23:34
@reubeno
Copy link
Contributor Author

reubeno commented Apr 4, 2025

No worries! Splitting the commits makes total sense. Thanks for merging it 😄

Thanks also for the pointer to eza's use of vmactions. Doing something similar sounds like a reasonable follow-up step at some point.

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