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

chore(gnoclient): Add Run support #1574

Merged
merged 2 commits into from
Jan 25, 2024

Conversation

jefft0
Copy link
Contributor

@jefft0 jefft0 commented Jan 23, 2024

gnoclient already has Call which does the same operation as gnokey maketx call. This PR adds Run to do the same thing as gnokey maketx run. It closely follows the implementation in gnokey.

(When this PR is merged, in Gno Native Kit we will add the thin wrapper for Run, similar to the wrapper for Call. We need this so that a command-line app can run a test script on the blockchain to, for example, stress test by adding hundreds of messages to r/demo/boards in a single call. It would be too slow to make hundreds of separate calls.)

To test, I called Run with the test script in this gist. As expected, the result is:

# HELLO WORLD!
## users.Render("")
 * [test_1](/r/demo/users:test_1)


## for i < 10 {print "hey hey hey!"}
0. hey hey hey!
1. hey hey hey!
2. hey hey hey!
3. hey hey hey!
4. hey hey hey!
5. hey hey hey!
6. hey hey hey!
7. hey hey hey!
8. hey hey hey!
9. hey hey hey!

## tests.* & std.*
- `tests.CurrentRealmPath` gno.land/r/demo/tests
- `tests.IsOriginCall` false
- `tests.GetPrevRealm` (struct{("g1a9r05e34s7dxe7wh8v00lnewnxtfnfq02gn5ws" std.Address),("gno.land/r/g1jg8mtutu9khhfwc4nxmuhcpftf0pajdhfvsqf5/run" string)} std.Realm)
- `std.GetOrigCaller` g1jg8mtutu9khhfwc4nxmuhcpftf0pajdhfvsqf5
- `std.PrevRealm` (struct{("g1jg8mtutu9khhfwc4nxmuhcpftf0pajdhfvsqf5" std.Address),("" string)} std.Realm)

## stateful check
- before: 0
- after: 10

## bf
source:  ++++++++++[>+++++++>++++++++++>+++>+<<<<-]>++.>+.+++++++..+++.>++.<<+++++++++++++++.>.+++.------.--------.
result:  Hello World

## complex types
- before:  foobar
- after:  foobar_modified

@jefft0 jefft0 requested a review from moul as a code owner January 23, 2024 13:12
@github-actions github-actions bot added the 📦 ⛰️ gno.land Issues or PRs gno.land package related label Jan 23, 2024
Copy link

codecov bot commented Jan 23, 2024

Codecov Report

Attention: 15 lines in your changes are missing coverage. Please review.

Comparison is base (17b1303) 55.73% compared to head (e3b7c8a) 55.96%.
Report is 12 commits behind head on master.

Files Patch % Lines
gno.land/pkg/gnoclient/client_txs.go 67.39% 10 Missing and 5 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1574      +/-   ##
==========================================
+ Coverage   55.73%   55.96%   +0.23%     
==========================================
  Files         435      435              
  Lines       65933    66742     +809     
==========================================
+ Hits        36748    37354     +606     
- Misses      26303    26465     +162     
- Partials     2882     2923      +41     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Signed-off-by: Jeff Thompson <jeff@thefirst.org>
Copy link
Member

@moul moul left a comment

Choose a reason for hiding this comment

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

That looks good.

Can you add a unit test, or an integration one?
If needed, @gfanton can probably give you some hints.

Signed-off-by: Jeff Thompson <jeff@thefirst.org>
@jefft0
Copy link
Contributor Author

jefft0 commented Jan 24, 2024

@moul, I added a test for Run. To keep the expected output small, it just uses one of the tests from the example gist. Is that right, or do you want the test script to be bigger? (This is testing the gnoclient Run method, not the back end VM support for Run.)

@thehowl thehowl merged commit f398c56 into gnolang:master Jan 25, 2024
180 of 182 checks passed
thehowl pushed a commit that referenced this pull request Jan 25, 2024
## Description

This PR fixes the `noop` log reference that was broken in
#1574, after the #1302 PR was merged
in earlier.

<details><summary>Contributors' checklist...</summary>

- [x] Added new tests, or not needed, or not feasible
- [x] Provided an example (e.g. screenshot) to aid review or the PR is
self-explanatory
- [x] Updated the official documentation or not needed
- [x] No breaking changes were made, or a `BREAKING CHANGE: xxx` message
was included in the description
- [x] Added references to related issues and PRs
- [ ] Provided any useful hints for running manual tests
- [ ] Added new benchmarks to [generated
graphs](https://gnoland.github.io/benchmarks), if any. More info
[here](https://github.com/gnolang/gno/blob/master/.benchmarks/README.md).
</details>
leohhhn pushed a commit to leohhhn/gno that referenced this pull request Jan 31, 2024
gnoclient [already has
`Call`](https://github.com/gnolang/gno/blob/7339bdcde85de886bf7069cc4adaf31e6d48f9cd/gno.land/pkg/gnoclient/client_txs.go#L25)
which does the same operation as `gnokey maketx call`. This PR adds
`Run` to do the same thing as `gnokey maketx run`. It closely follows
the [implementation in
gnokey](https://github.com/gnolang/gno/blob/7339bdcde85de886bf7069cc4adaf31e6d48f9cd/gno.land/pkg/keyscli/run.go#L111-L139).

(When this PR is merged, in Gno Native Kit we will add the thin wrapper
for Run, similar to the wrapper for Call. We need this so that a
command-line app can run a test script on the blockchain to, for
example, stress test by adding hundreds of messages to r/demo/boards in
a single call. It would be too slow to make hundreds of separate calls.)

To test, I called `Run` with the test script in [this
gist](https://gist.github.com/moul/ccf1e2aff64e7a1f0c5ca5e2d98d7e9a). As
expected, the result is:
```
# HELLO WORLD!
## users.Render("")
 * [test_1](/r/demo/users:test_1)


## for i < 10 {print "hey hey hey!"}
0. hey hey hey!
1. hey hey hey!
2. hey hey hey!
3. hey hey hey!
4. hey hey hey!
5. hey hey hey!
6. hey hey hey!
7. hey hey hey!
8. hey hey hey!
9. hey hey hey!

## tests.* & std.*
- `tests.CurrentRealmPath` gno.land/r/demo/tests
- `tests.IsOriginCall` false
- `tests.GetPrevRealm` (struct{("g1a9r05e34s7dxe7wh8v00lnewnxtfnfq02gn5ws" std.Address),("gno.land/r/g1jg8mtutu9khhfwc4nxmuhcpftf0pajdhfvsqf5/run" string)} std.Realm)
- `std.GetOrigCaller` g1jg8mtutu9khhfwc4nxmuhcpftf0pajdhfvsqf5
- `std.PrevRealm` (struct{("g1jg8mtutu9khhfwc4nxmuhcpftf0pajdhfvsqf5" std.Address),("" string)} std.Realm)

## stateful check
- before: 0
- after: 10

## bf
source:  ++++++++++[>+++++++>++++++++++>+++>+<<<<-]>++.>+.+++++++..+++.>++.<<+++++++++++++++.>.+++.------.--------.
result:  Hello World

## complex types
- before:  foobar
- after:  foobar_modified
```

---------

Signed-off-by: Jeff Thompson <jeff@thefirst.org>
leohhhn pushed a commit to leohhhn/gno that referenced this pull request Jan 31, 2024
## Description

This PR fixes the `noop` log reference that was broken in
gnolang#1574, after the gnolang#1302 PR was merged
in earlier.

<details><summary>Contributors' checklist...</summary>

- [x] Added new tests, or not needed, or not feasible
- [x] Provided an example (e.g. screenshot) to aid review or the PR is
self-explanatory
- [x] Updated the official documentation or not needed
- [x] No breaking changes were made, or a `BREAKING CHANGE: xxx` message
was included in the description
- [x] Added references to related issues and PRs
- [ ] Provided any useful hints for running manual tests
- [ ] Added new benchmarks to [generated
graphs](https://gnoland.github.io/benchmarks), if any. More info
[here](https://github.com/gnolang/gno/blob/master/.benchmarks/README.md).
</details>
@leohhhn leohhhn changed the title chore: Add Run to gnoclient chore(gnoclient): Add Run support Feb 8, 2024
@jefft0 jefft0 deleted the chore/add-Run-to-gnoclient branch March 4, 2024 08:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
📦 ⛰️ gno.land Issues or PRs gno.land package related
Projects
Status: Done
Status: Done
Archived in project
Development

Successfully merging this pull request may close these issues.

3 participants