gno icon indicating copy to clipboard operation
gno copied to clipboard

feat(gnokey): Print out the transaction hash when maketx executes successfully

Open linhpn99 opened this issue 1 year ago • 6 comments

Relate to https://github.com/gnolang/gno/issues/2303

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.

linhpn99 avatar Jun 08 '24 03:06 linhpn99

Codecov Report

Attention: Patch coverage is 0% with 4 lines in your changes missing coverage. Please review.

Project coverage is 55.00%. Comparing base (1180def) to head (79d5f22).

Files Patch % Lines
tm2/pkg/crypto/keys/client/broadcast.go 0.00% 2 Missing :warning:
tm2/pkg/crypto/keys/client/maketx.go 0.00% 2 Missing :warning:
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #2309      +/-   ##
==========================================
- Coverage   55.01%   55.00%   -0.01%     
==========================================
  Files         595      595              
  Lines       79727    79731       +4     
==========================================
  Hits        43858    43858              
- Misses      32550    32554       +4     
  Partials     3319     3319              
Flag Coverage Δ
contribs/gnodev 26.00% <ø> (ø)
contribs/gnofaucet 14.46% <ø> (-0.86%) :arrow_down:
contribs/gnokeykc 0.00% <ø> (ø)
contribs/gnomd 0.00% <ø> (ø)
gno.land 64.24% <ø> (ø)
tm2 54.45% <0.00%> (+0.01%) :arrow_up:

Flags with carried forward coverage won't be shown. Click here to find out more.

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

codecov[bot] avatar Jun 08 '24 03:06 codecov[bot]

@linhpn99 can you please merge in the master branch? 🙏

zivkovicmilos avatar Jun 12 '24 16:06 zivkovicmilos

Can we have at least one unit test not checking that "a string is printed", but actually checking that the hash looks correct?

moul avatar Jun 12 '24 16:06 moul

Can we have at least one unit test not checking that "a string is printed", but actually checking that the hash looks correct?

for sure, i'm working on it

linhpn99 avatar Jun 12 '24 17:06 linhpn99

Can we have at least one unit test not checking that "a string is printed", but actually checking that the hash looks correct?

However, with the current code structure, it is not possible to write unit tests due to the need to mock certain components (like rpcClient). I think it would be better to create a separate PR to address this issue, which would include the necessary mocking and test cases for the missing functions. wdyt @moul @zivkovicmilos ?

linhpn99 avatar Jun 13 '24 01:06 linhpn99

Can we have at least one unit test not checking that "a string is printed", but actually checking that the hash looks correct?

However, with the current code structure, it is not possible to write unit tests due to the need to mock certain components (like rpcClient). I think it would be better to create a separate PR to address this issue, which would include the necessary mocking and test cases for the missing functions. wdyt @moul @zivkovicmilos ? I means this implementation will not happen in this PR

linhpn99 avatar Jun 13 '24 09:06 linhpn99