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(gnovm): gno test now has gas used information #2571

Open
wants to merge 14 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 8 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 16 additions & 8 deletions gnovm/cmd/gno/test.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import (
"github.com/gnolang/gno/tm2/pkg/errors"
"github.com/gnolang/gno/tm2/pkg/random"
"github.com/gnolang/gno/tm2/pkg/std"
"github.com/gnolang/gno/tm2/pkg/store"
"github.com/gnolang/gno/tm2/pkg/testutils"
)

Expand Down Expand Up @@ -194,7 +195,7 @@ func execTest(cfg *testCfg, args []string, io commands.IO) error {
sort.Strings(pkg.FiletestGnoFiles)

startedAt := time.Now()
err = gnoTestPkg(pkg.Dir, pkg.TestGnoFiles, pkg.FiletestGnoFiles, cfg, io)
gasUsed, err := gnoTestPkg(pkg.Dir, pkg.TestGnoFiles, pkg.FiletestGnoFiles, cfg, io)
duration := time.Since(startedAt)
dstr := fmtDuration(duration)

Expand All @@ -205,7 +206,7 @@ func execTest(cfg *testCfg, args []string, io commands.IO) error {
io.ErrPrintfln("FAIL")
testErrCount++
} else {
io.ErrPrintfln("ok %s \t%s", pkg.Dir, dstr)
io.ErrPrintfln("ok %s \ttotal gas used: %d", pkg.Dir, gasUsed)
Copy link
Contributor

Choose a reason for hiding this comment

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

Why can't we have both gas used and duration? Both seem useful.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I see that people are not often concern about that time so much.

Copy link
Contributor

@deelawn deelawn Jul 23, 2024

Choose a reason for hiding this comment

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

I'll ask in another way -- how do we benefit from removing it? If someone doesn't care about it then they don't have to use it. If someone does care about it then they have it. It's only a few more characters being printed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@deelawn
I am fine with keeping the duration to be displayed.
But I have another view, should we move duration to -print-runtime-metrics which currently displaying runtime: cycle=%s imports=%d allocs=%s?

Copy link
Contributor

Choose a reason for hiding this comment

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

Before these changes, only subpackage level test execution displayed duration. The runtime metrics are something different. I'm only suggesting that we don't remove existing functionality if there is no good reason to do so. I don't think duration needs to be added anywhere else it doesn't currently exist.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You are right 👍 . I re-added the executing duration back again.
The result now looks like this:
image

}
}
if testErrCount > 0 || buildErrCount > 0 {
Expand All @@ -222,7 +223,7 @@ func gnoTestPkg(
filetestFiles []string,
cfg *testCfg,
io commands.IO,
) error {
) (int64, error) {
var (
verbose = cfg.verbose
rootDir = cfg.rootDir
Expand All @@ -234,7 +235,7 @@ func gnoTestPkg(
stderr = io.Err()
errs error
)

gasUsed := int64(0)
thinhnx-var marked this conversation as resolved.
Show resolved Hide resolved
mode := tests.ImportModeStdlibsOnly
if cfg.withNativeFallback {
// XXX: display a warn?
Expand Down Expand Up @@ -271,7 +272,7 @@ func gnoTestPkg(
})

if hasError {
return commands.ExitCodeError(1)
return gasUsed, commands.ExitCodeError(1)
}
testPkgName := getPkgNameFromFileset(ifiles)

Expand All @@ -287,6 +288,8 @@ func gnoTestPkg(
}

m := tests.TestMachine(testStore, stdout, gnoPkgPath)
// initial new gasMeter
m.GasMeter = store.NewGasMeter(10000 * 1000 * 1000)
thinhnx-var marked this conversation as resolved.
Show resolved Hide resolved
if printRuntimeMetrics {
// from tm2/pkg/sdk/vm/keeper.go
// XXX: make maxAllocTx configurable.
Expand All @@ -295,7 +298,10 @@ func gnoTestPkg(
m.Alloc = gno.NewAllocator(maxAllocTx)
}
m.RunMemPackage(memPkg, true)
bGasUsed := m.GasMeter.GasConsumed()
thinhnx-var marked this conversation as resolved.
Show resolved Hide resolved
err := runTestFiles(m, tfiles, memPkg.Name, verbose, printRuntimeMetrics, runFlag, io)
aGasUsed := m.GasMeter.GasConsumed()
gasUsed += (aGasUsed - bGasUsed)
if err != nil {
errs = multierr.Append(errs, err)
}
Expand Down Expand Up @@ -357,7 +363,9 @@ func gnoTestPkg(
}

testFilePath := filepath.Join(pkgPath, testFileName)
err := tests.RunFileTest(rootDir, testFilePath, tests.WithSyncWanted(cfg.updateGoldenTests))
var err error
gasUsedInThisPeriod, err := tests.RunFileTest(rootDir, testFilePath, tests.WithSyncWanted(cfg.updateGoldenTests))
gasUsed += gasUsedInThisPeriod
duration := time.Since(startedAt)
dstr := fmtDuration(duration)

Expand All @@ -375,13 +383,13 @@ func gnoTestPkg(
}

if verbose {
io.ErrPrintfln("--- PASS: %s (%s)", testName, dstr)
io.ErrPrintfln("--- PASS: %s (%s) with GasUsed: %d", testName, dstr, gasUsedInThisPeriod)
}
// XXX: add per-test metrics
}
}

return errs
return gasUsed, errs
}

// attempts to determine the full gno pkg path by analyzing the directory.
Expand Down
10 changes: 7 additions & 3 deletions gnovm/stdlibs/testing/testing.gno
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@ import (
"os"
"strconv"
"strings"
// gno "github.com/gnolang/gno/gnovm/pkg/gnolang"
// "github.com/gnolang/gno/tm2/pkg/store"
)

// ----------------------------------------
Expand Down Expand Up @@ -326,7 +328,9 @@ func tRunner(t *T, fn testingFunc, verbose bool) {
if !t.shouldRun(t.name) {
return
}

// m := gno.NewMachine("", nil)
thinhnx-var marked this conversation as resolved.
Show resolved Hide resolved
// m.GasMeter = store.NewGasMeter(10000 * 1000 * 1000)
// gasStart := 0
start := unixNano()

defer func() {
Expand All @@ -341,15 +345,15 @@ func tRunner(t *T, fn testingFunc, verbose bool) {

dur := unixNano() - start
t.dur = formatDur(dur)

// gasUsed := m.GasMeter.GasConsumed()
if t.verbose {
switch {
case t.Failed():
fmt.Fprintf(os.Stderr, "--- FAIL: %s (%s)\n", t.name, t.dur)
case t.skipped:
fmt.Fprintf(os.Stderr, "--- SKIP: %s (%s)\n", t.name, t.dur)
case t.verbose:
fmt.Fprintf(os.Stderr, "--- PASS: %s (%s)\n", t.name, t.dur)
fmt.Fprintf(os.Stderr, "--- PASS: %s \n", t.name)
}
}
}()
Expand Down
20 changes: 13 additions & 7 deletions gnovm/tests/file.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import (
osm "github.com/gnolang/gno/tm2/pkg/os"
"github.com/gnolang/gno/tm2/pkg/sdk"
"github.com/gnolang/gno/tm2/pkg/std"
"github.com/gnolang/gno/tm2/pkg/store/types"
"github.com/pmezard/go-difflib/difflib"
)

Expand Down Expand Up @@ -103,8 +104,9 @@ func WithSyncWanted(v bool) RunFileTestOption {

// RunFileTest executes the filetest at the given path, using rootDir as
// the directory where to find the "stdlibs" directory.
func RunFileTest(rootDir string, path string, opts ...RunFileTestOption) error {
func RunFileTest(rootDir string, path string, opts ...RunFileTestOption) (int64, error) {
var f runFileTestOptions
gasUsed := int64(0)
thinhnx-var marked this conversation as resolved.
Show resolved Hide resolved
for _, opt := range opts {
opt(&f)
}
Expand All @@ -124,7 +126,9 @@ func RunFileTest(rootDir string, path string, opts ...RunFileTestOption) error {
store := TestStore(rootDir, "./files", stdin, stdout, stderr, mode)
store.SetLogStoreOps(true)
m := testMachineCustom(store, pkgPath, stdout, maxAlloc, send)

// set a gasMeter for machine that runs the tests, consider the limit of this
thinhnx-var marked this conversation as resolved.
Show resolved Hide resolved
m.GasMeter = types.NewGasMeter(10000 * 1000 * 1000)
thinhnx-var marked this conversation as resolved.
Show resolved Hide resolved
beforeGas := m.GasMeter.GasConsumed()
// TODO support stdlib groups, but make testing safe;
// e.g. not be able to make network connections.
// interp.New(interp.Options{GoPath: goPath, Stdout: &stdout, Stderr: &stderr})
Expand All @@ -133,7 +137,7 @@ func RunFileTest(rootDir string, path string, opts ...RunFileTestOption) error {
// m.Use(unsafe.Symbols)
bz, err := os.ReadFile(path)
if err != nil {
return err
return gasUsed, err
}
{ // Validate result, errors, etc.
var pnc interface{}
Expand Down Expand Up @@ -279,7 +283,7 @@ func RunFileTest(rootDir string, path string, opts ...RunFileTestOption) error {

// NOTE: ignores any gno.GetDebugErrors().
gno.ClearDebugErrors()
return nil // nothing more to do.
return gasUsed, nil // nothing more to do.
} else {
// record errors when errWanted is empty and pnc not nil
if pnc != nil {
Expand Down Expand Up @@ -307,7 +311,7 @@ func RunFileTest(rootDir string, path string, opts ...RunFileTestOption) error {
panic(fmt.Sprintf("fail on %s: got unexpected debug error(s): %v", path, gno.GetDebugErrors()))
}
// pnc is nil, errWanted empty, no gno debug errors
return nil
return gasUsed, nil
}
case "Output":
// panic if got unexpected error
Expand Down Expand Up @@ -374,10 +378,12 @@ func RunFileTest(rootDir string, path string, opts ...RunFileTestOption) error {
}
}
default:
return nil
return gasUsed, nil
}
}
}
afterGas := m.GasMeter.GasConsumed()
gasUsed = afterGas - beforeGas

// Check that machine is empty.
err = m.CheckEmpty()
Expand All @@ -387,7 +393,7 @@ func RunFileTest(rootDir string, path string, opts ...RunFileTestOption) error {
}
panic(fmt.Sprintf("fail on %s: machine not empty after main: %v", path, err))
}
return nil
return gasUsed, nil
}

func wantedFromComment(p string) (directives []string, pkgPath, res, err, rops string, maxAlloc int64, send std.Coins) {
Expand Down
3 changes: 2 additions & 1 deletion gnovm/tests/file_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -137,8 +137,9 @@ func runFileTest(t *testing.T, path string, opts ...RunFileTestOption) {
logger = t.Log
}
rootDir := filepath.Join("..", "..")
err := RunFileTest(rootDir, path, append(opts, WithLoggerFunc(logger))...)
_, err := RunFileTest(rootDir, path, append(opts, WithLoggerFunc(logger))...)
if err != nil {
t.Fatalf("got error: %v", err)
}
// fmt.Printf("GasUsed runFileTest: %v\n", gasUsed)
thinhnx-var marked this conversation as resolved.
Show resolved Hide resolved
}
Loading