Skip to content
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

adding tests for state_creator of import_object #865

Conversation

YaronWittenstein
Copy link
Contributor

Part of the PR #807 changes was adding support for shared import objects between threads.

https://github.com/wasmerio/wasmer/pull/807/files#diff-d20cb4c5a883566b85be4cc046f45aa9R49

I've added tests/examples on how to create an import object with a state_creator
(function or closure)

@YaronWittenstein YaronWittenstein force-pushed the import-object-state-creator-tests branch from 20acb96 to 170ffa2 Compare October 7, 2019 12:09
Copy link
Contributor

@MarkMcCaskey MarkMcCaskey left a comment

Choose a reason for hiding this comment

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

Thanks! looks good, just a few small things (the lint is the only thing blocking this from shipping)

@@ -1,4 +1,4 @@
#![deny(
#![allow(
Copy link
Contributor

Choose a reason for hiding this comment

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

This should be changed back!

Copy link
Contributor Author

@YaronWittenstein YaronWittenstein Oct 7, 2019

Choose a reason for hiding this comment

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

@MarkMcCaskey my bad


#[test]
fn state_creator_closure() {
let ptr = std::ptr::null();
Copy link
Contributor

Choose a reason for hiding this comment

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

Perhaps we should use a sentinel value, 0 is a common bit pattern in memory, something like:

let ptr = unsafe { std::mem::transmute(0xB7B7A0A01234) };

would make the test more robust I think

@YaronWittenstein YaronWittenstein force-pushed the import-object-state-creator-tests branch from 343463c to 4adfbc4 Compare October 7, 2019 19:03
@MarkMcCaskey
Copy link
Contributor

bors r+

bors bot added a commit that referenced this pull request Nov 25, 2019
865: adding tests for `state_creator` of `import_object` r=MarkMcCaskey a=YaronWittenstein

Part of the PR #807 changes was adding support for shared import objects between threads.

https://github.com/wasmerio/wasmer/pull/807/files#diff-d20cb4c5a883566b85be4cc046f45aa9R49

I've added tests/examples on how to create an `import object` with a state_creator
(function or closure)

Co-authored-by: Yaron Wittenstein <[email protected]>
@bors
Copy link
Contributor

bors bot commented Nov 25, 2019

Build failed

@YaronWittenstein
Copy link
Contributor Author

bors r+

@MarkMcCaskey can you please re-trigger bors ?

@MarkMcCaskey
Copy link
Contributor

bors r+

bors bot added a commit that referenced this pull request Dec 2, 2019
865: adding tests for `state_creator` of `import_object` r=MarkMcCaskey a=YaronWittenstein

Part of the PR #807 changes was adding support for shared import objects between threads.

https://github.com/wasmerio/wasmer/pull/807/files#diff-d20cb4c5a883566b85be4cc046f45aa9R49

I've added tests/examples on how to create an `import object` with a state_creator
(function or closure)

Co-authored-by: Yaron Wittenstein <[email protected]>
@MarkMcCaskey
Copy link
Contributor

Thanks! Sorry about not shipping this before -- I only very recently learned/realized that not everyone can use bors on Wasmer! I assumed given an approval, people could ship their own PRs, but this seems to not be the case! There are a few other PRs I didn't ship due to this misunderstanding which I'll be following up on

@bors
Copy link
Contributor

bors bot commented Dec 2, 2019

Build failed

@MarkMcCaskey
Copy link
Contributor

bors r+

bors bot added a commit that referenced this pull request Dec 6, 2019
709: new feature flag: `deterministic` r=MarkMcCaskey a=YaronWittenstein

The motivation for the PR is for introducing a new feature flag called `deterministic`.

When `deterministic` will be enabled (turned-off by default) it'll guarantee deterministic
execution of wasm programs across different hardware/circumstances.

This is critical for Blockchain projects that require execution to be deterministic
in order to reach a consensus of the state transition of each smart-contract transaction.

865: adding tests for `state_creator` of `import_object` r=MarkMcCaskey a=YaronWittenstein

Part of the PR #807 changes was adding support for shared import objects between threads.

https://github.com/wasmerio/wasmer/pull/807/files#diff-d20cb4c5a883566b85be4cc046f45aa9R49

I've added tests/examples on how to create an `import object` with a state_creator
(function or closure)

1042: Make regression test work in release builds too. r=nlewycky a=nlewycky

Fix this regression test to detect the bug it was looking for in release builds too.

This bug triggered an assertion failure in debug, and by examining the pre-opt IR, we can check for the bug in release mode too.


Co-authored-by: Yaron Wittenstein <[email protected]>
Co-authored-by: Yaron Wittenstein <[email protected]>
Co-authored-by: Mark McCaskey <[email protected]>
Co-authored-by: Mark McCaskey <[email protected]>
Co-authored-by: Nick Lewycky <[email protected]>
@bors
Copy link
Contributor

bors bot commented Dec 6, 2019

Build failed (retrying...)

bors bot added a commit that referenced this pull request Dec 6, 2019
865: adding tests for `state_creator` of `import_object` r=MarkMcCaskey a=YaronWittenstein

Part of the PR #807 changes was adding support for shared import objects between threads.

https://github.com/wasmerio/wasmer/pull/807/files#diff-d20cb4c5a883566b85be4cc046f45aa9R49

I've added tests/examples on how to create an `import object` with a state_creator
(function or closure)

1042: Make regression test work in release builds too. r=nlewycky a=nlewycky

Fix this regression test to detect the bug it was looking for in release builds too.

This bug triggered an assertion failure in debug, and by examining the pre-opt IR, we can check for the bug in release mode too.


Co-authored-by: Yaron Wittenstein <[email protected]>
Co-authored-by: Nick Lewycky <[email protected]>
@bors
Copy link
Contributor

bors bot commented Dec 6, 2019

Build failed (retrying...)

bors bot added a commit that referenced this pull request Dec 6, 2019
865: adding tests for `state_creator` of `import_object` r=MarkMcCaskey a=YaronWittenstein

Part of the PR #807 changes was adding support for shared import objects between threads.

https://github.com/wasmerio/wasmer/pull/807/files#diff-d20cb4c5a883566b85be4cc046f45aa9R49

I've added tests/examples on how to create an `import object` with a state_creator
(function or closure)

Co-authored-by: Yaron Wittenstein <[email protected]>
@bors
Copy link
Contributor

bors bot commented Dec 6, 2019

Build failed

@syrusakbary
Copy link
Member

bors r+

@wasmerio wasmerio deleted a comment from bors bot Mar 31, 2020
bors bot added a commit that referenced this pull request Mar 31, 2020
865: adding tests for `state_creator` of `import_object` r=syrusakbary a=YaronWittenstein

Part of the PR #807 changes was adding support for shared import objects between threads.

https://github.com/wasmerio/wasmer/pull/807/files#diff-d20cb4c5a883566b85be4cc046f45aa9R49

I've added tests/examples on how to create an `import object` with a state_creator
(function or closure)

Co-authored-by: Yaron Wittenstein <[email protected]>
Co-authored-by: Syrus Akbary <[email protected]>
@bors
Copy link
Contributor

bors bot commented Mar 31, 2020

Build failed

@syrusakbary
Copy link
Member

This is no longer needed on the refactor. Adding the label so we can close quickly the PR after publishing the refactor.

@syrusakbary
Copy link
Member

State creator is no longer supported in the import object.
We moved towards a cleaner API in the refactor, removing the need for state creation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants