fix(sdk): wing test fails if unable to clean up files

[Chriscbr]

If a container mounts a volume to the state directory as described in #6295, it can sometimes result in permission errors when the CLI is cleaning up files after wing test because the mounted directory may have different permissions.

EACCES: permission denied, scandir 'target/test/mount.test.wsim/.state/c892787d9270cdccc06c34c95f7f9738be92b19134'

This error only seems to happen on CI for me. I'm not sure of the cause but I'm guessing that in CI, docker runs in a different (rootless maybe?) mode than on macOS.

Unblocks winglang/winglibs#207

Checklist

• Title matches Winglang's style guide
• Description explains motivation and solution
• Tests added (always)
• Docs updated (only required for features)
• Added pr/e2e-full label if this feature requires end-to-end testing

By submitting this pull request, I confirm that my contribution is made under the terms of the Wing Cloud Contribution License.

[github-actions]

Thanks for opening this pull request! 🎉
Please consult the contributing guidelines for details on how to contribute to this project.
If you need any assistence, don't hesitate to ping the relevant owner over Slack.

Topic	Owner
Wing SDK and utility APIs	@chriscbr
Wing Console	@ainvoner, @skyrpex, @polamoros
JSON, structs, primitives and collections	@hasanaburayyan
Platforms and plugins	@hasanaburayyan
Frontend resources (website, react, etc)	@tsuf239
Language design	@chriscbr
VSCode extension and language server	@markmcculloh
Compiler architecture, inflights, lifting	@yoav-steinberg
Wing Testing Framework	@tsuf239
Wing CLI	@markmcculloh
Build system, dev environment, releases	@markmcculloh
Library Ecosystem	@chriscbr
Documentation	@hasanaburayyan
SDK test suite	@tsuf239
Examples	@hasanaburayyan
Wing Playground	@eladcon

[monadabot]

Console preview environment is available at https://wing-console-pr-6320.fly.dev 🚀

Last Updated (UTC) 2024-04-25 13:35

[monadabot]

Benchmarks

Comparison to Baseline ⬜⬜⬜⬜🟥⬜⬜⬜⬜⬜⬜⬜⬜

Benchmark	Before	After	Change
version	62ms±3.34	63ms±0.93	+1ms (+1.04%)⬜
functions_1.test.w -t sim	400ms±5.65	410ms±4.96	+10ms (+2.37%)⬜
functions_1.test.w -t tf-aws	820ms±6.53	844ms±16.91	+24ms (+2.95%)⬜
jsii_big.test.w -t sim	2806ms±17.37	2832ms±17.92	+26ms (+0.93%)⬜
jsii_big.test.w -t tf-aws	3006ms±13.91	3037ms±11.24	+31ms (+1.04%)🟥
functions_10.test.w -t sim	496ms±8.5	499ms±12.54	+3ms (+0.67%)⬜
functions_10.test.w -t tf-aws	2037ms±11.59	2035ms±10.05	-2ms (-0.09%)⬜
jsii_small.test.w -t sim	377ms±6.45	377ms±6.11	0ms (-0.1%)⬜
jsii_small.test.w -t tf-aws	624ms±8.77	611ms±6.63	-13ms (-2.08%)⬜
empty.test.w -t sim	368ms±3.44	371ms±3.46	+3ms (+0.89%)⬜
empty.test.w -t tf-aws	604ms±7.3	601ms±5.8	-4ms (-0.61%)⬜
hello_world.test.w -t sim	419ms±7.82	410ms±7.61	-9ms (-2.12%)⬜
hello_world.test.w -t tf-aws	1548ms±11.16	1537ms±9.64	-11ms (-0.69%)⬜

⬜ Within 1.5 standard deviations
🟩 Faster, Above 1.5 standard deviations
🟥 Slower, Above 1.5 standard deviations

Benchmarks may vary outside of normal expectations, especially when running in GitHub Actions CI.

Results

name	mean	min	max	moe	sd
version	63ms	62ms	66ms	1ms	1ms
functions_1.test.w -t sim	410ms	395ms	421ms	5ms	7ms
functions_1.test.w -t tf-aws	844ms	818ms	883ms	17ms	24ms
jsii_big.test.w -t sim	2832ms	2793ms	2875ms	18ms	25ms
jsii_big.test.w -t tf-aws	3037ms	3016ms	3058ms	11ms	16ms
functions_10.test.w -t sim	499ms	467ms	534ms	13ms	18ms
functions_10.test.w -t tf-aws	2035ms	2009ms	2056ms	10ms	14ms
jsii_small.test.w -t sim	377ms	362ms	388ms	6ms	9ms
jsii_small.test.w -t tf-aws	611ms	600ms	624ms	7ms	9ms
empty.test.w -t sim	371ms	361ms	377ms	3ms	5ms
empty.test.w -t tf-aws	601ms	588ms	613ms	6ms	8ms
hello_world.test.w -t sim	410ms	393ms	432ms	8ms	11ms
hello_world.test.w -t tf-aws	1537ms	1525ms	1571ms	10ms	13ms

Last Updated (UTC) 2024-04-25 13:40

[Chriscbr]

@MarkMcCulloh @eladb I wasn't sure what the best way to handle this was to be honest (docker and unix permissions inexperience). My first thought was "if my local Wing tests pass, but the test runner couldn't clean everything up, maybe we should just make it a warning" - that was the easiest solution to implement, so I did that here.

But there are some other options, I just wasn't sure which would be worth the investment at the moment:

• Keep raising an error if cleanup fails, but we'll try harder to clean up the files. If the rmSync call fails, the code could try to recursively change the permissions on all files within the output directory and then try deleting everything again.
• Change sim.Container so it doesn't create weirdly permissioned files by parsing the volumes array given to its constructor, looking for local file paths, and try to change the permissions after the bind mount is created.
• Try to change the approach from feat(sdk): support mounting sim.Container to state directories #6295 so instead of using bind mounts (mounting the container's file system directly to a place in your file system), we use named mounts. These are created with docker volume create pgdata and deleted with docker volume create pgdata and are used with a command like docker run -v pgdata:/var/lib/postgresql/data postgres:15. I'm not sure how to redesign sim.Container API around this though.

(These are all just ideas, I'm not sure which would work in practice)

[MarkMcCulloh]

I like the idea of using named volumes. It's a nice separation of something stateful and as a resource it would conveniently have its own lifecycle.
Not exactly sure how the APIs will look but it's the only solution that comes to mind that isn't hacky.

Side note: Volumes even seem like a worthy abstraction for the cloud namespace

[mergify]

Thanks for contributing, @Chriscbr! This PR will now be added to the merge queue, or immediately merged if rybickic/container-testing is up-to-date with main and the queue is empty.

[monadabot]

Congrats! 🚀 This was released in Wing 0.72.8.