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

Creating workspaces test suit #46

Merged
merged 20 commits into from
Sep 5, 2023
Merged

Conversation

jaswinder6991
Copy link
Contributor

@jaswinder6991 jaswinder6991 commented Jul 28, 2023

Resolves #35

wip PR for creating test suit using workspaces-rs

@jaswinder6991 jaswinder6991 marked this pull request as draft July 28, 2023 12:26
@jaswinder6991
Copy link
Contributor Author

Hi @frol, this is the WIP PR for workspaces tests. You can check the migrations issue which occurs if you self_upgrade with the same wasm code that is already deployed.
I can meanwhile write the test to check wasm incompatibility if any.

@jaswinder6991
Copy link
Contributor Author

Closes #35

@frol
Copy link
Collaborator

frol commented Jul 29, 2023

@jaswinder6991 The issue you observe is related to Wasm binaries produced by Rust 1.70.0 being not compatible with nearcore runtime:

Use the following rust-toolchain.toml in the root of the project and re-compile the contract:

[toolchain]
channel = "1.69.0"

The test runs successfully when the contract is compiled with Rust 1.69:

image

@jaswinder6991
Copy link
Contributor Author

Hi @frol, thanks for your comment. I was just testing the case where wasm was not 1.69.0 so sort of a negative case.
Anyway, I was able to deploy 1.71.0 compiled code on testnet. Not sure how?

So for closing #35, is this PR enough? Should I add some more tests?
I can work on adding this to github workflows.
@ailisp

Copy link
Collaborator

@frol frol left a comment

Choose a reason for hiding this comment

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

Here are the remaining items for this PR to land:

@jaswinder6991
Copy link
Contributor Author

Hi @frol
The latest commit works on the list of tasks you suggested.

  1. CI workflow already has cargo test check, the integration tests execute as a part of cargo test too. Do I need something additional.
  2. Tests can be run via "cargo test"
  3. I have added assert statements. These are however incomplete at the moment, pending resolving the upgrade issue mentioned in next point.
  4. I have successfully used both import_contract and compile_project in the tests. I am currently struggling with the below issue when I call the self_upgrade at line.
status: Failure(ActionError(ActionError { index: Some(0), kind: FunctionCallError(ExecutionError("Exceeded the prepaid gas.")) }

Once I am done with the Gas fees issue, I will update the PR. Any help with the issue is updated.

@jaswinder6991
Copy link
Contributor Author

After multiple attempts to solve the below issue (mentioned here in more details.).

status: Failure(ActionError(ActionError { index: Some(0), kind: FunctionCallError(ExecutionError("Exceeded the prepaid gas.")) }

I have written a helper function to mimic compile_project functionality. It does not do error handling as well as the workspaces library but it is working. The reason of above issue produced by workspaces-rs version [0.7.0] is still unknown. However, the workspaces-rs current unreleased codebase do have updates for this function. We can switch to using workspaces-rs if we solve the issue in the future. For noe we continue with the helper function.

@jaswinder6991 jaswinder6991 marked this pull request as ready for review August 6, 2023 12:04
tests/workspaces.rs Outdated Show resolved Hide resolved
tests/README.md Outdated
Comment on lines 1 to 8
### This subdirectory contains tests for the devhub smart contract. It uses workspaces-rs to create sandbox environment to deploy the contract and run tests.

Use the below command to run the test.

`` cargo test
``


Copy link
Collaborator

Choose a reason for hiding this comment

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

Can you make this README formatting look nicer?

@frol
Copy link
Collaborator

frol commented Aug 28, 2023

I am marking this PR as draft as CI still fails.

@frol frol marked this pull request as draft August 28, 2023 20:57
@jaswinder6991
Copy link
Contributor Author

I am marking this PR as draft as CI still fails.

The CI worked fine a few weeks ago. Now I am seeing the same Gas Exceeded Error for which I wrote the custom function.
Details mentioned here: #46 (comment)

I will look into it again. However, can you think of a reason why self_upgrade would fail with prepaid gas exceeded error.

@jaswinder6991
Copy link
Contributor Author

The Prepaid Gas exceeded issue occured due to lack of gas assigned in self_upgrade function to code deployment and function execution. This PR solves it: #53

@frol frol marked this pull request as ready for review September 5, 2023 12:53
Copy link
Collaborator

@frol frol left a comment

Choose a reason for hiding this comment

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

@jaswinder6991 I had refactored some bits of the changes, please, review them for your future reference.

@frol frol enabled auto-merge (squash) September 5, 2023 13:07
@frol frol merged commit 85241b6 into NEAR-DevHub:main Sep 5, 2023
1 check passed
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.

Add a workspace test to prevent incompatible wasm issue happend last week
2 participants