feat: fix gno test for _test.gno and _filetest.gno files
#945
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -136,9 +136,13 @@ func execTest(cfg *testCfg, args []string, io *commands.IO) error { | |||||
| cfg.rootDir = guessRootDir() | ||||||
| } | ||||||
|
|
||||||
| paths, err := gnoPackagesFromArgs(args) | ||||||
|
There was a problem hiding this comment. better to leave it as pkgPaths so as to differentiate from file paths. There was a problem hiding this comment. but they can be file paths too. e.g. Later I'm also planning to support patterns in args. e.g.
There was a problem hiding this comment. Let's do the renaming in a new PR so we can focus on names. Suggestion:
Suggested change
// targetsFromPatterns returns a list of local dirs or single files
// intended to be used by gno commands such as `gno test`.
func targetsFromPatterns(args []string) []target {
// TODO: support './...'
} |
||||||
| if err != nil { | ||||||
| return fmt.Errorf("list package paths from args: %w", err) | ||||||
| } | ||||||
| if len(paths) == 0 { | ||||||
| io.ErrPrintln("no packages to test") | ||||||
| return nil | ||||||
| } | ||||||
|
|
||||||
| if cfg.timeout > 0 { | ||||||
|
|
@@ -148,75 +152,72 @@ func execTest(cfg *testCfg, args []string, io *commands.IO) error { | |||||
| }() | ||||||
| } | ||||||
|
|
||||||
| subPkgs, err := gnomod.SubPkgsFromPaths(paths) | ||||||
|
There was a problem hiding this comment. we CAN have a function called "SubPkgsFromDir" (singular), but not "SubPkgsFromDirs" or "SubPkgsFromPath" or "SubPkgsFromPaths". There was a problem hiding this comment. Based on how The main difference is that they are only supporting dirs and not single file, while we want our CLI to basically support this at the end: I think we should consider that we have:
Edit: @harry-hov found this which is better: https://github.com/golang/go/blob/master/src/cmd/go/internal/load/pkg.go#L2816; it consists of looking for packages, but creating virtual packages of a single file when a file is passed instead of a directory or pattern. |
||||||
| if err != nil { | ||||||
| return fmt.Errorf("list sub packages: %w", err) | ||||||
| } | ||||||
|
|
||||||
| buildErrCount := 0 | ||||||
| testErrCount := 0 | ||||||
| for _, pkg := range subPkgs { | ||||||
| if cfg.precompile { | ||||||
| if verbose { | ||||||
| io.ErrPrintfln("=== PREC %s", pkg.Dir) | ||||||
|
There was a problem hiding this comment. my main point is that here, we should use pkg.Path, which is not the same as any Dir. because "PkgPath/pkgPath/pkgpath/pkg.Path" are all the same "package path" identifier, whereas anything that ends with "Dir" is a filesystem location. They can diverge. Also a package path starts with a domain unless it is special, like a stdlibs package or something special like "main". Dir is a different beast based on the filesystem. Either absolute or relative, not something that starts with "gno.land" nor "main". It might be "." or "./something" though, or "/something". We should ideally make sure all Dirs start with a dot or slash. There was a problem hiding this comment. I agree we should print pkg.Path instead of Dir. original var Will fix this. But in another PR. (Out of scope for this PR) |
||||||
| } | ||||||
| precompileOpts := newPrecompileOptions(&precompileCfg{ | ||||||
| output: tempdirRoot, | ||||||
| }) | ||||||
| err := precompilePkg(importPath(pkg.Dir), precompileOpts) | ||||||
|
There was a problem hiding this comment. ditto. importPath should take pkg.Path. There was a problem hiding this comment. ok I see that importPath is declared as a there can be some function that converts a package path to a file path, Other than such a function, we shouldn't be converting package path Names to file paths or dir paths with type conversions, because nobody should be doing such a thing without calling the official function for such conversions (above). An "import path" I don't see why we need to create such a name, because it is nothing more than package paths. So ImportPkgPaths is less confusing. Or just Imports of Names. Any path that is not a package path, should be clearly named as such, by calling it a "file path" or a "dir path" that is a string, never a Name type. Anything like package path, pkgPath, pkg.Path, are all package paths. If you want to associate a file path or a dir path it will have to be called pkg.FilePath or pkg.DirPath. There was a problem hiding this comment.
Yeah, |
||||||
| if err != nil { | ||||||
| io.ErrPrintln(err) | ||||||
| io.ErrPrintln("FAIL") | ||||||
| io.ErrPrintfln("FAIL %s", pkg.Dir) | ||||||
| io.ErrPrintln("FAIL") | ||||||
|
|
||||||
| buildErrCount++ | ||||||
| continue | ||||||
| } | ||||||
|
|
||||||
| if verbose { | ||||||
| io.ErrPrintfln("=== BUILD %s", pkg.Dir) | ||||||
| } | ||||||
| tempDir, err := ResolvePath(tempdirRoot, importPath(pkg.Dir)) | ||||||
| if err != nil { | ||||||
| return errors.New("cannot resolve build dir") | ||||||
| } | ||||||
| err = goBuildFileOrPkg(tempDir, defaultBuildOptions) | ||||||
| if err != nil { | ||||||
| io.ErrPrintln(err) | ||||||
| io.ErrPrintln("FAIL") | ||||||
| io.ErrPrintfln("FAIL %s", pkg.Dir) | ||||||
| io.ErrPrintln("FAIL") | ||||||
|
|
||||||
| buildErrCount++ | ||||||
| continue | ||||||
| } | ||||||
| } | ||||||
|
|
||||||
| if len(pkg.TestGnoFiles) == 0 && len(pkg.FiletestGnoFiles) == 0 { | ||||||
| io.ErrPrintfln("? %s \t[no test files]", pkg.Dir) | ||||||
| continue | ||||||
| } | ||||||
|
|
||||||
| sort.Strings(pkg.TestGnoFiles) | ||||||
|
There was a problem hiding this comment. GnoTests string There was a problem hiding this comment. or TestFiles, FiletestFiles? |
||||||
| sort.Strings(pkg.FiletestGnoFiles) | ||||||
|
|
||||||
| startedAt := time.Now() | ||||||
| err = gnoTestPkg(pkg.Dir, pkg.TestGnoFiles, pkg.FiletestGnoFiles, cfg, io) | ||||||
| duration := time.Since(startedAt) | ||||||
| dstr := fmtDuration(duration) | ||||||
|
|
||||||
| if err != nil { | ||||||
| io.ErrPrintfln("%s: test pkg: %v", pkg.Dir, err) | ||||||
| io.ErrPrintfln("FAIL") | ||||||
| io.ErrPrintfln("FAIL %s \t%s", pkg.Dir, dstr) | ||||||
| io.ErrPrintfln("FAIL") | ||||||
| testErrCount++ | ||||||
| } else { | ||||||
| io.ErrPrintfln("ok %s \t%s", pkg.Dir, dstr) | ||||||
| } | ||||||
| } | ||||||
| if testErrCount > 0 || buildErrCount > 0 { | ||||||
|
|
||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this usage of Dir is OK since you need to file location to load.