-
Notifications
You must be signed in to change notification settings - Fork 277
Allow platform-dependent snapshot test results #1270
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
fbd8edb
to
f25f0e9
Compare
we need to support different output on different platforms due to various differences that we can't paper over. for example, macOS defines some builtins with different types than Linux, and macOS libc and glibc have different typedef setups for fixed-size types, resulting in different type alias preambles. support these by appending the operating system to the snapshot name for results that may vary by platform.
1505315
to
f5a4af3
Compare
@@ -0,0 +1 @@ | |||
snapshots__transpile-linux@rotate.c.snap No newline at end of file |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What generates this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure whether you mean what creates this string or what creates the file [email protected]
. The string itself is from me running ln -s
to introduce the symlink. The target file is created by insta itself inside insta::glob
if multiple files are globbed, along with other files for the other glob matches.
If insta::glob
only matches one file, it puts it at a path with no @input.filename
portion, hence needing the symlink.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, these are symlinks. insta
does that removing common prefix thing here again. I'd rather just put a dummy empty.c
file in snapshots/platform-specific
, and then once we add another real test we can remove it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
|
||
let status = Command::new("rustc") | ||
.args(&["--crate-type", "lib", "--edition", "2021"]) | ||
.args(&["--crate-type", "lib", "--edition", "2021", "-o", "-"]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah this is a lot simpler way of doing this. This writes to stout? How does that work vs /dev/null
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/dev/null
would be a more obvious bit-bucket, but rustc attempts to create temporary files in the output directory and can't for /dev
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What gets printed to stdout with -
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The --emit=link
contents, whatever they are. In this case the output is not captured, so it doesn't really matter what it is.
@@ -0,0 +1,28 @@ | |||
--- |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How do we generate the macOS snapshots on Linux easily?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's no way to do so from Linux without a macOS VM; we need to compile against Apple's proprietary (though nominally open-source) headers. If we want to support macOS, this is the price we pay.
We could improve the workflow by having snapshot .new files produced as an artifact from our macOS CI pipeline.
insta handily uses different snapshot file naming schemes dependent on whether a glob finds one or multiple files. as we only have one platform-dependent test yet, it doesn't include the @filename suffix, but it will when we add more. for now, add a dummy test so that insta always uses the `@file.c` suffix; we should delete the dummy test when we have more platform-specific tests.
f5a4af3
to
5044813
Compare
This is needed for #1266.