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

feat: add grc20factory example #1913

Merged
merged 3 commits into from
May 30, 2024
Merged

Conversation

moul
Copy link
Member

@moul moul commented Apr 10, 2024

Signed-off-by: moul 94029+moul@users.noreply.github.com

Contributors' checklist...
  • Added new tests, or not needed, or not feasible
  • Provided an example (e.g. screenshot) to aid review or the PR is self-explanatory
  • Updated the official documentation or not needed
  • No breaking changes were made, or a BREAKING CHANGE: xxx message was included in the description
  • Added references to related issues and PRs
  • Provided any useful hints for running manual tests
  • Added new benchmarks to generated graphs, if any. More info here.

Signed-off-by: moul <94029+moul@users.noreply.github.com>
@moul moul self-assigned this Apr 10, 2024
@github-actions github-actions bot added the 🧾 package/realm Tag used for new Realms or Packages. label Apr 10, 2024
@moul
Copy link
Member Author

moul commented Apr 10, 2024

@leohhhn, here's a contract factory example. I haven't included safe objects due to my preference for improving the grc20 API first. I plan to create a new factory example for safe objects without grc20 to focus on redesigning grc20 thoroughly.

@gfanton Do we now support txtar files directly in the same folder as .txtar, or should we keep them separate from .gno files? I want to include more usage examples with maketx without creating more invalid location files.

Copy link

codecov bot commented Apr 10, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 47.78%. Comparing base (ca6566f) to head (4cb6713).
Report is 8 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1913      +/-   ##
==========================================
- Coverage   47.79%   47.78%   -0.01%     
==========================================
  Files         393      393              
  Lines       61654    61663       +9     
==========================================
+ Hits        29465    29466       +1     
- Misses      29717    29725       +8     
  Partials     2472     2472              

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

@gfanton
Copy link
Member

gfanton commented Apr 10, 2024

@moul For now, the only constraint is to create a Go test file that will run testscript with a target directory, which can be any directory (current directory or testdata). We use testdata because it's what Go actually uses to run their .txtar files, but there is no universal convention.
A simple but efficient solution could be to create a single top-level Go test file that recursively iterates through the example folder and executes any testdata (or even any directory, but I don't think it's a good idea) it encounters. So, any realm could have its own testdata folder without the need to centralize .txtar somewhere.
wdyt ?

@moul
Copy link
Member Author

moul commented Apr 11, 2024

@moul For now, the only constraint is to create a Go test file that will run testscript with a target directory, which can be any directory (current directory or testdata). We use testdata because it's what Go actually uses to run their .txtar files, but there is no universal convention. A simple but efficient solution could be to create a single top-level Go test file that recursively iterates through the example folder and executes any testdata (or even any directory, but I don't think it's a good idea) it encounters. So, any realm could have its own testdata folder without the need to centralize .txtar somewhere. wdyt ?

I think that txtar tests should be a built-in feature that follows a standard convention, like file extension.

I prefer not to create Go files solely for configuring a developer pipeline. I prefer that when encountering a Go file in a realm, I know it requires my attention.

cc @gfanton

@leohhhn leohhhn mentioned this pull request Apr 15, 2024
Signed-off-by: moul <94029+moul@users.noreply.github.com>
@moul moul marked this pull request as ready for review April 21, 2024 12:05
@moul moul requested review from a team as code owners April 21, 2024 12:05
@moul moul mentioned this pull request Apr 29, 2024
7 tasks
@thehowl
Copy link
Member

thehowl commented May 2, 2024

@moul It seems to me you want to have a testable example which works with maketx call, for instance.

I would prefer if testscript / txtar remained an internal tool. Discussing with @gfanton on the review meeting, we may still benefit from unifying how txtar works (rather than having two differing impl's in gnoland and cmd/gno), and having it as a command (which we can still get coverage information for).

However, long-term, I don't like to use txtar as documentation. While they are decently straightforward to use, thanks to @gfanton's acute design taste :), I think we should try to have something better to make testable examples.

An idea that sprung up, was to have markdown files for documentation of a realm, with the code blocks being testable.

@gfanton will create further issues on this.

@moul
Copy link
Member Author

moul commented May 2, 2024

@moul It seems to me you want to have a testable example which works with maketx call, for instance.

Testable examples are my ideal testing method. 👍

I would prefer if testscript / txtar remained an internal tool. Discussing with @gfanton on the review meeting, we may still benefit from unifying how txtar works (rather than having two differing impl's in gnoland and cmd/gno), and having it as a command (which we can still get coverage information for).

Keeping this testing method more internal is a good goal. Currently, the only way to test advanced logic is by writing scripts in gno.land/cmd/gnoland. Enhancing the tool to directly run txtar files from packages' folder would be an improvement. Making this feature "internal" could involve not documenting it in the main documentation, allowing advanced users to use it.

An idea that sprung up, was to have markdown files for documentation of a realm, with the code blocks being testable.

Yes, or alternatively, provide testable examples and adhere to documentation standards that include comments in .gno files.

@moul
Copy link
Member Author

moul commented May 20, 2024

I believe this PR should be merged. Txtar enhancements can be addressed in a separate issue or PR.

@thehowl thehowl merged commit 0d299c3 into gnolang:master May 30, 2024
11 checks passed
omarsy pushed a commit to TERITORI/gno that referenced this pull request Jun 3, 2024
Signed-off-by: moul <94029+moul@users.noreply.github.com>

<!-- please provide a detailed description of the changes made in this
pull request. -->

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

- [ ] Added new tests, or not needed, or not feasible
- [ ] Provided an example (e.g. screenshot) to aid review or the PR is
self-explanatory
- [ ] Updated the official documentation or not needed
- [ ] No breaking changes were made, or a `BREAKING CHANGE: xxx` message
was included in the description
- [ ] 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>

---------

Signed-off-by: moul <94029+moul@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🧾 package/realm Tag used for new Realms or Packages.
Projects
Status: Done
Status: Done
Status: ✅ Done
Status: Done
Development

Successfully merging this pull request may close these issues.

4 participants