Skip to content

Conversation

@Alexhuszagh
Copy link
Contributor

@Alexhuszagh Alexhuszagh commented Jul 14, 2022

Adds an environment variable CROSS_SANDBOXED that skips tests that touch the filesystem if set.

Closes #943.

@Alexhuszagh Alexhuszagh added the no changelog A valid PR without changelog (no-changelog) label Jul 14, 2022
@Alexhuszagh Alexhuszagh requested a review from a team as a code owner July 14, 2022 20:03
@Alexhuszagh
Copy link
Contributor Author

I think using an #[ignore] attribute is the best solution, since we still want to be able to run the tests if desired on NixOS, and there might be other scenarios where access to the filesystem is denied other than just NixOS. Adding it to the build script feels a bit overkill.

@Emilgardis
Copy link
Member

Emilgardis commented Jul 14, 2022

I wish there was a way to make ignore configurable with envvars or something. I'm fine with this, however I'd prefer it to be not ignored unconditionally. A runtime skip maybe?

@Alexhuszagh
Copy link
Contributor Author

Alexhuszagh commented Jul 14, 2022

I wish there was a way to make ignore configurable with envvars or something. I'm fine with this, however I'd prefer it to be not ignored unconditionally. A runtime skip maybe?

Sure, we could look for NIX_PATH, which is always set. And we could check for another envvar to not skip it if present. This way, it can still be tested. The other way is to add it based on the envvar to the build script, and do #[cfg_attr(nixos, ignore)]. That's probably the better solution, but I initially wanted to skip that.

@Alexhuszagh
Copy link
Contributor Author

Alexhuszagh commented Jul 14, 2022

Ok I've added a check for NIX_PATH in the build script, however, this will not work in interesting cases if the build script somehow isn't run on NixOS but the actual code is compiled on NixOS. However, I think that's a rare enough possibility that this should be fine since it's only in a unit test for this to be true, would have to be outside the nix-build setting.

@Emilgardis
Copy link
Member

worst case for that, just pass --ignored

@Emilgardis Emilgardis requested a review from otavio July 14, 2022 21:32
@Alexhuszagh
Copy link
Contributor Author

Alexhuszagh commented Jul 14, 2022

@otavio I've added the CROSS_SANDBOXED environment variable, which if set skips any tests that touch the filesystem. Seem good? This is checked in the build script.

@Alexhuszagh Alexhuszagh changed the title Ignore test_host by default. Ignore tests that touch the filesystem if sandboxed. Jul 14, 2022
Adds an environment variable `CROSS_SANDBOXED` that skips tests that
touch the filesystem if set.
@Alexhuszagh
Copy link
Contributor Author

Alexhuszagh commented Jul 19, 2022

@otavio Does this look good? Does the use of an environment variable work for you, since this is quite a useful test to ensure our mount paths are correct?

@otavio
Copy link
Contributor

otavio commented Jul 19, 2022

It should work; I cannot try it today but will do once I have an opportunity.

Copy link
Contributor

@otavio otavio left a comment

Choose a reason for hiding this comment

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

We can merge this and I handle this in NixOS side.

@Alexhuszagh
Copy link
Contributor Author

bors r=Emilgardis,otavio

@bors
Copy link
Contributor

bors bot commented Jul 19, 2022

Build succeeded:

@bors bors bot merged commit bcbc012 into cross-rs:main Jul 19, 2022
@Emilgardis Emilgardis added this to the v0.3.0 milestone Aug 10, 2022
@Alexhuszagh Alexhuszagh added enhancement A-nixos-host Area: NixOS hosts no-ci-targets PRs that do not affect or should skip any cross-compilation targets. labels Nov 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-nixos-host Area: NixOS hosts enhancement no changelog A valid PR without changelog (no-changelog) no-ci-targets PRs that do not affect or should skip any cross-compilation targets.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Unit tests failing during NixOS build

3 participants