这是indexloc提供的服务,不要输入任何密码
Skip to content

scope or since implies include dependencies and no dependents #844

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

Closed
Closed
Show file tree
Hide file tree
Changes from all 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
11 changes: 9 additions & 2 deletions cli/internal/run/run.go
Original file line number Diff line number Diff line change
Expand Up @@ -717,6 +717,7 @@ func parseRunArgs(args []string, output cli.Ui) (*RunOptions, error) {
// it's only possible to set to true, but in the future a user could theoretically set
// it to false and override the default behavior.
includDepsSet := false
noDepsSet := false
for argIndex, arg := range args {
if arg == "--" {
runOptions.passThroughArgs = args[argIndex+1:]
Expand Down Expand Up @@ -754,6 +755,7 @@ func parseRunArgs(args []string, output cli.Ui) (*RunOptions, error) {

case strings.HasPrefix(arg, "--no-deps"):
runOptions.includeDependents = false
noDepsSet = true
case strings.HasPrefix(arg, "--no-cache"):
runOptions.cache = false
case strings.HasPrefix(arg, "--cacheFolder"):
Expand Down Expand Up @@ -809,8 +811,13 @@ func parseRunArgs(args []string, output cli.Ui) (*RunOptions, error) {
}
}
}
if len(runOptions.scope) != 0 && runOptions.since != "" && !includDepsSet {
runOptions.includeDependencies = true
if len(runOptions.scope) != 0 || runOptions.since != "" {
if !includDepsSet {
runOptions.includeDependencies = true
}
if !noDepsSet {
runOptions.includeDependents = false
}
}

// Force streaming output in CI/CD non-interactive mode
Expand Down
6 changes: 3 additions & 3 deletions cli/internal/run/run_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -62,12 +62,12 @@ func TestParseConfig(t *testing.T) {
"scope",
[]string{"foo", "--scope=foo", "--scope=blah"},
&RunOptions{
includeDependents: true,
includeDependents: false,
stream: true,
bail: true,
dotGraph: "",
concurrency: 10,
includeDependencies: false,
includeDependencies: true,
cache: true,
forceExecution: false,
profile: "",
Expand Down Expand Up @@ -150,7 +150,7 @@ func TestParseConfig(t *testing.T) {
"since and scope imply including dependencies for backwards compatibility",
[]string{"foo", "--scope=bar", "--since=some-ref"},
&RunOptions{
includeDependents: true,
includeDependents: false,
stream: true,
bail: true,
concurrency: 10,
Expand Down
1 change: 1 addition & 0 deletions cli/internal/scope/scope.go
Original file line number Diff line number Diff line change
Expand Up @@ -142,6 +142,7 @@ func ResolvePackages(opts *Opts, scm scm.SCM, ctx *context.Context, tui cli.Ui,
logger.Debug("running with dependents")
}

// ordered after includeDependents so that we pick up the dependencies of our dependents
if includeDependencies {
// TODO(gsoltis): we're concurrently iterating and adding to a map, potentially
// resulting in a bunch of duplicate work as we look for dependencies of something
Expand Down
65 changes: 40 additions & 25 deletions cli/internal/scope/scope_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -146,17 +146,21 @@ func TestResolvePackages(t *testing.T) {
includeDependents bool
}{
{
name: "One package changed",
changed: []string{"libs/libB/src/index.ts"},
expected: []string{"libB"},
since: "dummy",
name: "One package changed",
changed: []string{"libs/libB/src/index.ts"},
expected: []string{"libB", "libD"},
since: "dummy",
includeDependencies: true,
includeDependents: false,
},
{
name: "An ignored package changed",
changed: []string{"libs/libB/src/index.ts"},
expected: []string{},
since: "dummy",
ignore: "libs/libB/**/*.ts",
name: "An ignored package changed",
changed: []string{"libs/libB/src/index.ts"},
expected: []string{},
since: "dummy",
ignore: "libs/libB/**/*.ts",
includeDependencies: true,
includeDependents: false,
},
{
// nothing in scope depends on the change
Expand All @@ -166,6 +170,7 @@ func TestResolvePackages(t *testing.T) {
since: "dummy",
scope: []string{"app1"},
includeDependencies: true, // scope implies include-dependencies
includeDependents: false,
},
{
// a dependent lib changed, scope implies include-dependencies,
Expand All @@ -176,6 +181,7 @@ func TestResolvePackages(t *testing.T) {
since: "dummy",
scope: []string{"app1"},
includeDependencies: true, // scope implies include-dependencies
includeDependents: false,
},
{
// a dependent lib changed, user explicitly asked to not build dependencies
Expand All @@ -186,6 +192,7 @@ func TestResolvePackages(t *testing.T) {
since: "dummy",
scope: []string{"app1"},
includeDependencies: false,
includeDependents: false,
},
{
// a nested dependent lib changed, user explicitly asked to not build dependencies
Expand All @@ -196,28 +203,34 @@ func TestResolvePackages(t *testing.T) {
since: "dummy",
scope: []string{"app1"},
includeDependencies: false,
includeDependents: false,
},
{
name: "global dependency changed, even though it was ignored, forcing a build of everything",
changed: []string{"libs/libB/src/index.ts"},
expected: []string{"app0", "app1", "app2", "libA", "libB", "libC", "libD"},
since: "dummy",
ignore: "libs/libB/**/*.ts",
globalDeps: []string{"libs/**/*.ts"},
name: "global dependency changed, even though it was ignored, forcing a build of everything",
changed: []string{"libs/libB/src/index.ts"},
expected: []string{"app0", "app1", "app2", "libA", "libB", "libC", "libD"},
since: "dummy",
ignore: "libs/libB/**/*.ts",
globalDeps: []string{"libs/**/*.ts"},
includeDependents: false,
includeDependencies: true,
},
{
name: "an app changed, user asked for dependencies to build",
changed: []string{"app/app2/src/index.ts"},
since: "dummy",
includeDependencies: true,
includeDependents: false,
expected: []string{"app2", "libB", "libC", "libD"},
},
{
name: "a library changed, user asked for dependents to be built",
changed: []string{"libs/libB"},
since: "dummy",
includeDependents: true,
expected: []string{"app0", "app1", "app2", "libA", "libB"},
name: "a library changed, user asked for dependents to be built",
changed: []string{"libs/libB"},
since: "dummy",
includeDependents: true,
includeDependencies: true,
// note that libC is pulled in as a dependency of a dependent
expected: []string{"app0", "app1", "app2", "libA", "libB", "libC", "libD"},
},
{
// no changes, no base to compare against, defaults to everything
Expand All @@ -235,13 +248,15 @@ func TestResolvePackages(t *testing.T) {
scope: []string{"libB"},
expected: []string{"libB", "libD"},
includeDependencies: true, // scope implies include-dependencies
includeDependents: false,
},
{
name: "library change, no scope",
changed: []string{"libs/libA/src/index.ts"},
expected: []string{"libA", "app0", "app1"},
includeDependents: true,
since: "dummy",
name: "library change, no scope",
changed: []string{"libs/libA/src/index.ts"},
expected: []string{"libA", "libB", "libD", "app0", "app1"},
includeDependents: true,
includeDependencies: true,
since: "dummy",
},
}
for i, tc := range testCases {
Expand Down