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.

Sign up for GitHub

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

Jump to bottom

Fix & update rust tester #727

Merged
merged 3 commits into from
Jun 10, 2024
Merged

Fix & update rust tester #727

merged 3 commits into from
Jun 10, 2024

Conversation

sparkplug0025
Copy link
Contributor

Depends on #725 and #726

@sparkplug0025 sparkplug0025 mentioned this pull request May 28, 2024
Closed
swatanabe
swatanabe reviewed May 28, 2024
View reviewed changes
rust/cargo-psibase/src/link.rs Outdated Show resolved Hide resolved
rust/psibase/src/package.rs Outdated Show resolved Hide resolved
James-Mart
James-Mart reviewed May 29, 2024
View reviewed changes
rust/cargo-psibase/tester_wasi_polyfill/src/lib.rs Outdated Show resolved Hide resolved
rust/psibase/src/block.rs Outdated
#[to_key(psibase_mod = "crate")]
#[graphql(input_name = "BlockInfoInput")]
// #[to_key(psibase_mod = "crate")]
// #[graphql(input_name = "BlockInfoInput")]
Copy link
Member

Choose a reason for hiding this comment

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

Why eliminate these derives and attributes? Anyway I think you should just delete all these things instead of leaving them as comments.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Because the graphql didnt accept the new Consensus variant if the block is a Graphql InputObject for a query...I will remove the comments and create an issue to track it back.

We must have had a reason to have these blocks as input objects but honestly I didnt find these "BlockInfoInput" anywhere in the codebase... Thats why I removed for now

rust/cargo-psibase/src/link.rs Outdated Show resolved Hide resolved
@James-Mart James-Mart added the Dev Experience Related to the experience of developers label May 31, 2024
@sparkplug0025 sparkplug0025 force-pushed the sparkplug0025/rust-tester-update branch from 1f42257 to b67403e Compare June 3, 2024 11:19
@James-Mart
Copy link
Member

When I try to run cargo-psibase test on test_contract_2/, I get:

   Compiling example v0.10.0 (/root/psibase/rust/test_contract_2)
    Finished `release` profile [optimized] target(s) in 0.71s
 Polyfilling SKIPPED! No polyfill functions found
Reoptimizing example-e4db117f62b9711c.wasm
     Running example-e4db117f62b9711c.wasm

running 1 test
thread 'main' panicked at psibase/src/tester.rs:122:72:
called `Result::unwrap()` on an `Err` value: BadOffset
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
test test_arith ... vm::exception: unreachable
error: Failed running: psitest /root/psibase/rust/target/wasm32-wasi/release/deps/example-e4db117f62b9711c.wasm --nocapture

@James-Mart
Copy link
Member

Did you determine that 1.78 is now too old of a rust version?

$ rustc --version
rustc 1.78.0 (9b00956e5 2024-04-29)

@sparkplug0025
Copy link
Contributor Author

When I try to run cargo-psibase test on test_contract_2/, I get:

   Compiling example v0.10.0 (/root/psibase/rust/test_contract_2)
    Finished `release` profile [optimized] target(s) in 0.71s
 Polyfilling SKIPPED! No polyfill functions found
Reoptimizing example-e4db117f62b9711c.wasm
     Running example-e4db117f62b9711c.wasm

running 1 test
thread 'main' panicked at psibase/src/tester.rs:122:72:
called `Result::unwrap()` on an `Err` value: BadOffset
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
test test_arith ... vm::exception: unreachable
error: Failed running: psitest /root/psibase/rust/target/wasm32-wasi/release/deps/example-e4db117f62b9711c.wasm --nocapture

It's because we need to merge #726 first -- working on that

@sparkplug0025 sparkplug0025 force-pushed the sparkplug0025/rust-tester-update branch from 02a3100 to 32fcc63 Compare June 5, 2024 17:08
@sparkplug0025
Copy link
Contributor Author

Did you determine that 1.78 is now too old of a rust version?

$ rustc --version
rustc 1.78.0 (9b00956e5 2024-04-29)

As we talked about, no, the version 1.78 is good enough!

@James-Mart James-Mart mentioned this pull request Jun 5, 2024
Closed
James-Mart
James-Mart approved these changes Jun 5, 2024
View reviewed changes
Copy link
Member

@James-Mart James-Mart left a comment

Choose a reason for hiding this comment

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

I think there's some cleanup to the tester output that would be nice, but I documented it in a separate issue instead of holding up this PR.

rust/psibase/src/tester.rs
@@ -294,16 +273,27 @@ impl<T: fracpack::UnpackOwned> ChainResult<T> {
let ret = transact
.inner_traces
.iter()
// TODO: improve this filter.. we need to return whatever is the name of the action somehow if possible...
Copy link
Member

Choose a reason for hiding this comment

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

I think this comment is outdated. The output already does get the name of the action. Sample output:

>>> ChainResult::get - Inner action trace: setcpulimit (raw_retval=)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Problem is that we dont know which action name we should filter by...

@sparkplug0025 sparkplug0025 merged commit b8d2d94 into main Jun 10, 2024
4 checks passed
@sparkplug0025 sparkplug0025 deleted the sparkplug0025/rust-tester-update branch June 10, 2024 10:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@James-Mart James-Mart James-Mart approved these changes

@swatanabe swatanabe swatanabe approved these changes

Assignees
No one assigned
Labels
Dev Experience Related to the experience of developers
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@sparkplug0025 @James-Mart @swatanabe