Skip to content
This repository has been archived by the owner on Nov 9, 2019. It is now read-only.

Add a testcase for fd_filestat_set_* #56

Merged
merged 2 commits into from
Aug 9, 2019

Conversation

marmistrz
Copy link
Member

@marmistrz marmistrz commented Aug 9, 2019

This PR matches CraneStation/wasi-misc-tests#14 and should be merged before #54.
The file was created using cargo build --release --target=wasm32-wasi

(I'm not really sure how I should submit the .wasm files)

This PR matches CraneStation/wasi-misc-tests#14.
The file was created using `cargo build --release --target=wasm32-wasi`
@@ -136,6 +136,7 @@ cfg_if::cfg_if! {
"symlink_loop" => true,
"clock_time_get" => true,
"truncation_rights" => true,
"fd_filestat_set" => true,
Copy link
Member

Choose a reason for hiding this comment

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

I'm at a loss here. I thought that the whole point was to add a testcase that tests changes of #54 in addition to #47 :-D Anyhow, I suggest we wait with this PR after #54 is merge, or you could potentially add the test binary as part of #54.

@kubkon
Copy link
Member

kubkon commented Aug 9, 2019

This PR matches CraneStation/wasi-misc-tests#14 and should be merged before #54.
The file was created using cargo build --release --target=wasm32-wasi

(I'm not really sure how I should submit the .wasm files)

I'd personally submit the testcase as part of the PR which introduces a new feature, so in this case, #54. But this way is good provided we merge #54 first and we don't exclude the test from Windows build ;-)

@marmistrz
Copy link
Member Author

The idea was to guarantee that we don't accidentally change the semantics of fd_filestat_set_times. Now we test only #47, we'll enable this test on Windows as a part of #54.

@kubkon
Copy link
Member

kubkon commented Aug 9, 2019

Ah, that's an excellent point actually the results of which we have witnessed first hand in bytecodealliance/wasmtime#255 ;-)

@kubkon kubkon merged commit b9834e3 into CraneStation:master Aug 9, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants