Skip to content

Commit

Permalink
mod/modload: ignore _test.cue files in dependencies
Browse files Browse the repository at this point in the history
Currently files with a `_test.cue` suffix are recognized and ignored by
the `cue/load` code unless the Config.Tests field is true. However, as
there is no way to set that field from the command line, no way to
include a _test.cue file even when explicitly mentioned, and no
framework for writing tests in CUE, this is of very limited use.
However, it would be nice to provide proper test support in CUE in the
future, and the current behavior at least reserves a space in which that
can happen.

Thus, the dependency resolution code in mod/modload should match that of
cue/load, which ignores _test.cue files unless Config.Tests is true. It
seems like it would also be good to align with the current behavior of
_tool.cue files: they are ignored when outside the main module.

Also add a comment and make the logic a bit clearer
(shouldIncludePkgFile doesn't make a difference for packages in the main
module because they're added as part of the initial root set, ignoring
all build tags and including _test.cue and _tool.cue files.

Note: This differs from Go's treatment of test files: in Go, the
dependency resolution code includes enough dependencies to enable
running tests in any package directly or directly imported by the
non-test code, but does not include dependencies used for tests of
packages used only by external test code. For CUE, at least initially,
we will include only the dependencies needed by files in the main
module.

Fixes #3272.

Signed-off-by: Roger Peppe <rogpeppe@gmail.com>
Change-Id: Ifd3bb59652ef3fa4739817f1f551a58a8b1f5996
Reviewed-on: https://review.gerrithub.io/c/cue-lang/cue/+/1197605
Reviewed-by: Paul Jolly <paul@myitcv.io>
TryBot-Result: CUEcueckoo <cueckoo@cuelang.org>
  • Loading branch information
rogpeppe committed Jul 12, 2024
1 parent 77ac696 commit 7793367
Show file tree
Hide file tree
Showing 4 changed files with 153 additions and 12 deletions.
33 changes: 33 additions & 0 deletions cmd/cue/cmd/testdata/script/modtidy_with_build_attrs.txtar
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
# Check that, for the purposes of `cue mod tidy`, build attributes are always considered
# to be enabled in the main module but disabled in dependencies.
# Also check that _tool.cue and _test.cue files are ignored when they
# are part of external dependencies.

exec cue mod tidy
cmp cue.mod/module.cue want-module

Expand Down Expand Up @@ -104,6 +107,22 @@ self: "test.example/d1"
prodenabled: false
y: d4

-- _registry/test.example_d1_v0.0.1/a_tool.cue --

package d1

import "test.example/d5"

y: d5

-- _registry/test.example_d1_v0.0.1/a_test.cue --

package d1

import "test.example/d6"

y: d6

-- _registry/test.example_d2_v0.0.1/cue.mod/module.cue --
module: "test.example/d2"
language: version: "v0.9.2"
Expand All @@ -125,3 +144,17 @@ language: version: "v0.9.2"
-- _registry/test.example_d4_v0.0.1/x.cue --
package d4
self: "test.example/d4"

-- _registry/test.example_d5_v0.0.1/cue.mod/module.cue --
module: "test.example/d5"
language: version: "v0.9.2"
-- _registry/test.example_d5_v0.0.1/x.cue --
package d5
self: "test.example/d5"

-- _registry/test.example_d6_v0.0.1/cue.mod/module.cue --
module: "test.example/d6"
language: version: "v0.9.2"
-- _registry/test.example_d6_v0.0.1/x.cue --
package d6
self: "test.example/d6"
93 changes: 93 additions & 0 deletions cmd/cue/cmd/testdata/script/test_files.txtar
Original file line number Diff line number Diff line change
@@ -0,0 +1,93 @@
# _test.cue files should be ignored apart from for dependency analysis
# purposes. Note there is no way to use test files from the command line
# currently.

exec cue export
cmp stdout want-eval

# When tidying, the d3 dependency should not appear in the dependencies
# because it's only present in a _test.go file in an external
# dependency. The d2 dependency _should_ be present, because even though
# we don't currently support _test.go files, we still want to consider
# them as part of the main module.
exec cue mod tidy
cmp cue.mod/module.cue want-tidy

-- want-eval --
{
"x": {
"self": "d1"
}
}
-- want-tidy --
module: "test.example/main"
language: {
version: "v0.9.2"
}
deps: {
"test.example/d1@v0": {
v: "v0.0.1"
}
"test.example/d2@v0": {
v: "v0.0.1"
default: true
}
}
-- cue.mod/module.cue --
module: "test.example/main"
language: version: "v0.9.2"

deps: "test.example/d1": v: "v0.0.1"

-- main.cue --
package main

import "test.example/d1"

x: d1

-- main_test.cue --
package main

import "test.example/d2"

y: d3

-- _registry/test.example_d1_v0.0.1/cue.mod/module.cue --
module: "test.example/d1"
language: version: "v0.9.2"

-- _registry/test.example_d1_v0.0.1/x.cue --

package d1

self: "d1"

-- _registry/test.example_d1_v0.0.1/x_test.cue --

package d1

import "test.example/d3"

test: true
x: d3

-- _registry/test.example_d2_v0.0.1/cue.mod/module.cue --
module: "test.example/d2"
language: version: "v0.9.2"

-- _registry/test.example_d2_v0.0.1/x.cue --

package d2

self: "d2"

-- _registry/test.example_d3_v0.0.1/cue.mod/module.cue --
module: "test.example/d3"
language: version: "v0.9.2"

-- _registry/test.example_d3_v0.0.1/x.cue --

package d3

self: "d3"
15 changes: 12 additions & 3 deletions cue/load/instances.go
Original file line number Diff line number Diff line change
Expand Up @@ -235,12 +235,21 @@ func loadPackages(
if !cfg.Tools && strings.HasSuffix(mf.FilePath, "_tool.cue") {
return false
}
isTest := strings.HasSuffix(mf.FilePath, "_test.cue")
var tagIsSet func(string) bool
if mod.Path() == mainModPath || pkgPaths[pkgPath] {
if mod.Path() == mainModPath {
// In the main module.
if isTest && !cfg.Tests {
return false
}
tagIsSet = tg.tagIsSet
} else {
// The file is outside the main module and isn't mentioned explicitly
// on the command line, so treat all build tag keys as unset.
// Outside the main module.
if isTest {
// Don't traverse test files outside the main module
return false
}
// Treat all build tag keys as unset.
tagIsSet = func(string) bool {
return false
}
Expand Down
24 changes: 15 additions & 9 deletions internal/mod/modload/tidy.go
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,9 @@ func tidy(ctx context.Context, fsys fs.FS, modRoot string, reg Registry, checkTi
}
// TODO check that module path is well formed etc
origRs := modrequirements.NewRequirements(mf.QualifiedModule(), reg, mf.DepVersions(), mf.DefaultMajorVersions())
// Note: we can just ignore build tags and the fact that we might
// have _tool.cue and _test.cue files, because we want to include
// all of them.
rootPkgPaths, err := modimports.AllImports(modimports.AllModuleFiles(fsys, modRoot))
if err != nil {
return nil, err
Expand Down Expand Up @@ -173,15 +176,18 @@ func modfileFromRequirements(old *modfile.File, rs *modrequirements.Requirements
// In general a file should always be considered unless it's a _tool.cue file
// that's not in the main module.
func (ld *loader) shouldIncludePkgFile(pkgPath string, mod module.Version, fsys fs.FS, mf modimports.ModuleFile) bool {
inMainModule := mod.Path() == ld.mainModule.Path()
if strings.HasSuffix(mf.FilePath, "_tool.cue") {
// _tool.cue files are only considered when they are part of the main module.
return inMainModule
}
ok, _, err := buildattr.ShouldBuildFile(mf.Syntax, func(string) bool {
// Keys of build attributes are considered always true when they're
// in the main module and false otherwise.
return inMainModule
if mod.Path() == ld.mainModule.Path() {
// All files in the main module are considered.
return true
}
if strings.HasSuffix(mf.FilePath, "_tool.cue") || strings.HasSuffix(mf.FilePath, "_test.cue") {
// tool and test files are only considered when they are part of the main module.
return false
}
ok, _, err := buildattr.ShouldBuildFile(mf.Syntax, func(key string) bool {
// Keys of build attributes are considered always false when
// outside the main module.
return false
})
if err != nil {
return false
Expand Down

0 comments on commit 7793367

Please sign in to comment.