gno icon indicating copy to clipboard operation
gno copied to clipboard

build: update gnovm Makefile

Open notJoon opened this issue 2 years ago • 11 comments

Added stdlibs that were missing from _test.gnoland.pkg0

notJoon avatar Nov 14 '23 06:11 notJoon

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 47.77%. Comparing base (0c2d51e) to head (ec889e8).

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #1371   +/-   ##
=======================================
  Coverage   47.77%   47.77%           
=======================================
  Files         393      393           
  Lines       61630    61630           
=======================================
  Hits        29443    29443           
  Misses      29717    29717           
  Partials     2470     2470           

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

codecov[bot] avatar Nov 14 '23 06:11 codecov[bot]

would be nice to automate this, any idea?

moul avatar Nov 14 '23 11:11 moul

So, this one I'd jotted down to fix at some point; thank you for jumping ahead and doing it :)

My take on this:

  • Let's split the tests into TestPackages and TestPackagesLong.
  • TestPackagesLong tests regexp and bytes. Additionally, it is skipped on t.Short()
  • TestPackages, instead, tests all packages in stdlibs/*, except for those tested by -Long.
  • The Makefile is changed with the following
    • _test.gnolang.pkg: -run '^TestPackages$'
    • _test.gnolang.pkg.bytes: -run '^TestPackagesLong$/^bytes'
    • _test.gnolang.pkg.regexp: -run '^TestPackagesLong$/^regexp'. Note, this one should also test package regexp/syntax.

thehowl avatar Nov 14 '23 15:11 thehowl

would be nice to automate this, any idea?

It would definitely be convenient to automate, but for now I can only think of scanning the directory with a shell script(like find) and then dynamically generating test targets. I'm sure there's a better way, but I'll have to look into it. 🤔

notJoon avatar Nov 15 '23 06:11 notJoon

It would definitely be convenient to automate, but for now I can only think of scanning the directory with a shell script

You can just reorganise the tests (see my comment).

thehowl avatar Nov 15 '23 07:11 thehowl

Hey @notJoon,

Have you had the chance to implement @thehowl's suggestions? I guess we can go ahead and merge this fix for now, and leave the test separation for a later PR

zivkovicmilos avatar Nov 29 '23 11:11 zivkovicmilos

ping @notJoon

thehowl avatar Feb 22 '24 17:02 thehowl

ping @notJoon

Oh, I toatally forgot about this PR.

Is there anything else I need to work on? like the automation that we talked before.

notJoon avatar Feb 23 '24 01:02 notJoon

@notJoon see my comment for a proposal: https://github.com/gnolang/gno/pull/1371#issuecomment-1810496193

It requires adding an exception in code to simply split between "normal" and "long" packages; no automation / code-generation needed.

thehowl avatar Feb 23 '24 08:02 thehowl

It requires adding an exception in code to simply split between "normal" and "long" packages; no automation / code-generation needed.

Yes, I've updated to based on your comment, and I'm currently working on fixing the tests that aren't passing.

When I run it as an individual file it works fine, but when I run it with make I get the error interface {} is nil, not std.ExecContext. So, I'm trying to figure out why does it happend.

notJoon avatar Feb 28 '24 02:02 notJoon

When I run it as an individual file it works fine, but when I run it with make

I don't see the tests you're running. Feel free to ping me in DM with stacktraces/info and I can take a look.

From the looks of it, the place where you think the error is happening may be incorrect. It should be somewhere where we're doing an assertion on m.Context (like in gnovm/stdlibs/std/*.go files).

thehowl avatar Mar 04 '24 20:03 thehowl

@notJoon Can you merge master into this branch, and check the CI?

Please see @thehowl's comments

zivkovicmilos avatar Mar 29 '24 03:03 zivkovicmilos

After discussing with @notJoon privately, we decided to close out this PR 🙏

zivkovicmilos avatar May 17 '24 07:05 zivkovicmilos