-
Notifications
You must be signed in to change notification settings - Fork 5
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
Migrate tests to use python scripts instead of rust test helper #210
base: main
Are you sure you want to change the base?
Conversation
src/lib.rs
Outdated
r#" | ||
import sys | ||
print("foo", file=sys.stderr) | ||
"#, |
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.
If you use single quotes you can avoid raw strings:
r#" | |
import sys | |
print("foo", file=sys.stderr) | |
"#, | |
" | |
import sys | |
print('foo', file=sys.stderr) | |
", |
Having the code be out-of-line doesn't bother me so much, so I think I still prefer keeping it in Rust vs adding Python. That being said, I don't feel super strongly about it. I can definitely see the readability benefit of having the code be in-line. Overall the PR looks good. |
67ddf9b
to
1fba4bf
Compare
7402523
to
9db8dbb
Compare
This PR tries to get rid of the
test_executables_helper
in favor of specifying test programs in python. This has some advantages:match
statement insrc/test_executables/helper.rs
.rust-script
, even when the compilation artifacts are already cached.test_executables_helper
and thetest_executables
feature inCargo.toml
. I think these details ideally shouldn't be in that file.bitflags
dependency. Not sure though.There's also some disadvantages:
python3
to execute the tests. Fortunately, that is installed on lots of systems. For example to make this PR run on CI, I didn't have to install anything there.