From 6bb1bd9db61541f9102f28029f98dd61843036b9 Mon Sep 17 00:00:00 2001 From: Michael Dowling Date: Wed, 5 Apr 2023 16:15:16 -0700 Subject: [PATCH] Add git diff mode to CLI This commit adds support for running Smithy diff using a "git" mode so that the current state of a project is compared against the previous state of a project. By default, the previous state is the last commit of the current branch: ``` smithy diff --mode git ``` You can compare against a differet commit using: ``` smithy diff --mode git --old HEAD~2 smithy diff --mode git --old main ``` The way this works it that `git` mode is a wrapper around `project` mode. Smithy will create a git worktree for a specific branch named `__smithy-diff-worktree` located at "build/smithy/diff-worktree". Each time the command is run, the worktree is either created and pointed at the correct commit, or uses `git reset --hard $COMMIT`. When creating the worktree, we first call "git worktree prune" in case the worktree was previously created and deleted from a `smithy clean` operation. --- smithy-cli/build.gradle | 5 + .../amazon/smithy/cli/DiffCommandTest.java | 169 ++++++++++++++++-- .../amazon/smithy/cli/IntegUtils.java | 10 +- .../amazon/smithy/cli/HelpPrinter.java | 6 + .../smithy/cli/commands/BuildOptions.java | 21 +++ .../smithy/cli/commands/ClasspathAction.java | 12 +- .../smithy/cli/commands/DiffCommand.java | 109 +++++++++-- .../amazon/smithy/cli/HelpPrinterTest.java | 6 +- .../smithy/cli/commands/BuildOptionsTest.java | 36 ++++ 9 files changed, 342 insertions(+), 32 deletions(-) create mode 100644 smithy-cli/src/test/java/software/amazon/smithy/cli/commands/BuildOptionsTest.java diff --git a/smithy-cli/build.gradle b/smithy-cli/build.gradle index 748043ce32e..4f23cc5a4bd 100644 --- a/smithy-cli/build.gradle +++ b/smithy-cli/build.gradle @@ -229,6 +229,11 @@ task integ(type: Test) { testClassesDirs = sourceSets["it"].output.classesDirs classpath = sourceSets["it"].runtimeClasspath maxParallelForks = Runtime.getRuntime().availableProcessors() / 2 + + testLogging { + events = ["passed", "skipped", "failed"] + exceptionFormat = "full" + } } // Runtime images need to be created before integration tests can run. diff --git a/smithy-cli/src/it/java/software/amazon/smithy/cli/DiffCommandTest.java b/smithy-cli/src/it/java/software/amazon/smithy/cli/DiffCommandTest.java index 5708a784766..e429d445ab8 100644 --- a/smithy-cli/src/it/java/software/amazon/smithy/cli/DiffCommandTest.java +++ b/smithy-cli/src/it/java/software/amazon/smithy/cli/DiffCommandTest.java @@ -17,9 +17,7 @@ import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.containsString; -import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.is; -import static org.hamcrest.Matchers.not; import java.io.FileWriter; import java.io.IOException; @@ -27,7 +25,11 @@ import java.io.UncheckedIOException; import java.nio.file.Path; import java.nio.file.Paths; +import java.util.ArrayList; +import java.util.Collections; +import java.util.List; import org.junit.jupiter.api.Test; +import software.amazon.smithy.utils.IoUtils; import software.amazon.smithy.utils.ListUtils; public class DiffCommandTest { @@ -38,7 +40,7 @@ public void passingDiffEventsExitZero() { writeFile(a, "$version: \"2.0\"\nnamespace example\nstring A\n"); RunResult result = IntegUtils.run(dir, ListUtils.of("diff", "--old", a.toString(), "--new", a.toString())); - assertThat(result.getExitCode(), equalTo(0)); + assertThat("Not 0: output [" + result.getOutput() + ']', result.getExitCode(), is(0)); }); } @@ -49,7 +51,7 @@ public void showsLabelForOldModelEvents() { writeFile(a, "$version: \"2.0\"\nnamespace example\n@aaaaaa\nstring A\n"); RunResult result = IntegUtils.run(dir, ListUtils.of("diff", "--old", a.toString(), "--new", a.toString())); - assertThat(result.getExitCode(), equalTo(1)); + assertThat("Not 1: output [" + result.getOutput() + ']', result.getExitCode(), is(1)); assertThat(result.getOutput(), containsString("── OLD ERROR ──")); }); } @@ -64,7 +66,7 @@ public void showsLabelForNewModelEvents() { writeFile(b, "$version: \"2.0\"\nnamespace example\n@aaaaaa\nstring A\n"); RunResult result = IntegUtils.run(dir, ListUtils.of("diff", "--old", a.toString(), "--new", b.toString())); - assertThat(result.getExitCode(), equalTo(1)); + assertThat("Not 1: output [" + result.getOutput() + ']', result.getExitCode(), is(1)); assertThat(result.getOutput(), containsString("── NEW ERROR ──")); }); } @@ -83,7 +85,7 @@ public void showsLabelForDiffEvents() { "--old", a.toString(), "--new", b.toString(), "--severity", "NOTE")); // Note that this is required since the default severity is WARNING. - assertThat(result.getExitCode(), equalTo(0)); + assertThat("Not 0: output [" + result.getOutput() + ']', result.getExitCode(), is(0)); assertThat(result.getOutput(), containsString("── DIFF NOTE ──")); }); } @@ -110,7 +112,7 @@ public void doesNotUseImportsOrSourcesWithArbitraryMode() { "-c", dir.resolve("smithy-build.json").toString(), "--old", file.toString(), "--new", file.toString())); - assertThat(result.getExitCode(), equalTo(0)); + assertThat("Not 0: output [" + result.getOutput() + ']', result.getExitCode(), is(0)); }); } @@ -118,7 +120,7 @@ public void doesNotUseImportsOrSourcesWithArbitraryMode() { public void requiresOldAndNewForArbitraryMode() { RunResult result = IntegUtils.run(Paths.get("."), ListUtils.of("diff")); - assertThat(result.getExitCode(), is(1)); + assertThat("Not 1: output [" + result.getOutput() + ']', result.getExitCode(), is(1)); assertThat(result.getOutput(), containsString("Missing required --old argument")); } @@ -129,7 +131,7 @@ public void doesNotAllowNewWithProjectMode() { "--new", "x", "--old", "y")); - assertThat(result.getExitCode(), is(1)); + assertThat("Not 1: output [" + result.getOutput() + ']', result.getExitCode(), is(1)); assertThat(result.getOutput(), containsString("--new cannot be used with this diff mode")); } @@ -143,8 +145,9 @@ public void projectModeUsesConfigOfOldModel() { "project", "--old", outer.toString())); + + assertThat("Not 1: output [" + result.getOutput() + ']', result.getExitCode(), is(1)); assertThat(result.getOutput(), containsString("ChangedShapeType")); - assertThat(result.getExitCode(), equalTo(1)); }); }); } @@ -159,7 +162,151 @@ public void projectModeCanDiffAgainstSingleFile() { "project", "--old", dir.resolve("model").resolve("main.smithy").toString())); - assertThat(result.getExitCode(), equalTo(0)); + + assertThat("Not 0: output [" + result.getOutput() + ']', result.getExitCode(), is(0)); + }); + } + + @Test + public void gitDiffAgainstLastCommit() { + // Diff against itself (the only model file of the project), so there should be no differences. + IntegUtils.withProject("simple-config-sources", dir -> { + initRepo(dir); + commitChanges(dir); + + RunResult result = runDiff(dir); + + assertThat("Not 0: output [" + result.getOutput() + ']', result.getExitCode(), is(0)); + }); + } + + private RunResult runDiff(Path dir) { + return runDiff(dir, null); + } + + private RunResult runDiff(Path dir, String oldCommit) { + List args = new ArrayList<>(); + args.add("diff"); + args.add("--mode"); + args.add("git"); + if (oldCommit != null) { + args.add("--old"); + args.add(oldCommit); + } + + return IntegUtils.run(dir, args); + } + + @Test + public void canDiffAgainstLastCommitWithFailure() { + IntegUtils.withProject("simple-config-sources", dir -> { + initRepo(dir); + commitChanges(dir); + + Path file = dir.resolve("model").resolve("main.smithy"); + writeFile(file, "$version: \"2.0\"\nnamespace smithy.example\ninteger MyString\n"); + RunResult result = IntegUtils.run(dir, ListUtils.of("diff", "--mode", "git")); + + assertThat("Not 1: output [" + result.getOutput() + ']', result.getExitCode(), is(1)); + assertThat(result.getOutput(), containsString("ChangedShapeType")); + }); + } + + private void initRepo(Path dir) { + run(ListUtils.of("git", "init"), dir); + run(ListUtils.of("git", "config", "user.email", "you@example.com"), dir); + run(ListUtils.of("git", "config", "user.name", "Your Name"), dir); + } + + private void commitChanges(Path dir) { + run(ListUtils.of("git", "add", "-A"), dir); + run(ListUtils.of("git", "commit", "-m", "Foo"), dir); + } + + private void run(List args, Path root) { + StringBuilder output = new StringBuilder(); + int result = IoUtils.runCommand(args, root, output, Collections.emptyMap()); + if (result != 0) { + throw new RuntimeException("Error running command: " + args + ": " + output); + } + } + + @Test + public void gitDiffAgainstLastCommitAfterClean() { + IntegUtils.withProject("simple-config-sources", dir -> { + initRepo(dir); + commitChanges(dir); + RunResult result = runDiff(dir); + assertThat("Not zero: output [" + result.getOutput() + ']', result.getExitCode(), is(0)); + + // This will remove the previously created worktree. + IntegUtils.run(dir, ListUtils.of("clean")); + + // Running diff again ensures that we handle the case where there's a prunable worktree. + result = runDiff(dir); + + assertThat("Not zero: output [" + result.getOutput() + ']', result.getExitCode(), is(0)); + }); + } + + @Test + public void gitDiffEnsuresHeadUpdatesAsCommitsAreMade() { + IntegUtils.withProject("simple-config-sources", dir -> { + initRepo(dir); + commitChanges(dir); + + // Run with HEAD (the default) + RunResult result = runDiff(dir); + assertThat("Not zero: output [" + result.getOutput() + ']', result.getExitCode(), is(0)); + + // Now make another commit, which updates HEAD and ensures the worktree updates too. + Path file = dir.resolve("model").resolve("main.smithy"); + writeFile(file, "$version: \"2.0\"\nnamespace smithy.example\ninteger MyString\n"); + commitChanges(dir); + + // Run diff again, which won't fail because the last commit is the change that broke the model, meaning + // the current state of the model is valid. + result = runDiff(dir); + + assertThat("Not zero: output [" + result.getOutput() + ']', result.getExitCode(), is(0)); + }); + } + + @Test + public void gitDiffAgainstSpecificCommit() { + IntegUtils.withProject("simple-config-sources", dir -> { + initRepo(dir); + commitChanges(dir); + + // Run with explicit HEAD + RunResult result = runDiff(dir, "HEAD"); + assertThat("Not zero: output [" + result.getOutput() + ']', result.getExitCode(), is(0)); + + // Now make another commit, which updates HEAD and ensures the worktree updates too. + Path file = dir.resolve("model").resolve("main.smithy"); + writeFile(file, "$version: \"2.0\"\nnamespace smithy.example\ninteger MyString\n"); + commitChanges(dir); + + // Run diff again, which won't fail because the last commit is the change that broke the model, meaning + // the current state of the model is valid. + result = runDiff(dir, "HEAD~1"); + + assertThat("Not 1: output [" + result.getOutput() + ']', result.getExitCode(), is(1)); + assertThat(result.getOutput(), containsString("ChangedShapeType")); + }); + } + + @Test + public void gitModeDoesNotAllowNewArgument() { + IntegUtils.withProject("simple-config-sources", dir -> { + initRepo(dir); + commitChanges(dir); + + // Now that it's running in a git repo, it won't fail due to that and will validate that --new is invalid. + RunResult result = IntegUtils.run(dir, ListUtils.of("diff", "--mode", "git", "--new", "some-file.smithy")); + + assertThat("Not 1: output [" + result.getOutput() + ']', result.getExitCode(), is(1)); + assertThat(result.getOutput(), containsString("--new cannot be used with this diff mode")); }); } } diff --git a/smithy-cli/src/it/java/software/amazon/smithy/cli/IntegUtils.java b/smithy-cli/src/it/java/software/amazon/smithy/cli/IntegUtils.java index b37b73b329e..22015c527c0 100644 --- a/smithy-cli/src/it/java/software/amazon/smithy/cli/IntegUtils.java +++ b/smithy-cli/src/it/java/software/amazon/smithy/cli/IntegUtils.java @@ -33,11 +33,15 @@ import java.util.List; import java.util.Map; import java.util.function.Consumer; +import java.util.logging.Level; +import java.util.logging.Logger; import software.amazon.smithy.utils.IoUtils; import software.amazon.smithy.utils.MapUtils; public final class IntegUtils { + private static final Logger LOGGER = Logger.getLogger(IntegUtils.class.getName()); + private IntegUtils() {} public static void withProject(String projectName, Consumer consumer) { @@ -97,7 +101,11 @@ static void withTempDir(String name, Consumer consumer) { try { consumer.accept(path); } finally { - IoUtils.rmdir(path); + try { + IoUtils.rmdir(path); + } catch (Exception e) { + LOGGER.log(Level.INFO, "Unable to delete temp directory", e); + } } } catch (IOException e) { throw new UncheckedIOException(e); diff --git a/smithy-cli/src/main/java/software/amazon/smithy/cli/HelpPrinter.java b/smithy-cli/src/main/java/software/amazon/smithy/cli/HelpPrinter.java index 52d6f188c28..cb84e44e948 100644 --- a/smithy-cli/src/main/java/software/amazon/smithy/cli/HelpPrinter.java +++ b/smithy-cli/src/main/java/software/amazon/smithy/cli/HelpPrinter.java @@ -260,6 +260,12 @@ LineWrapper space() { LineWrapper appendWithinLine(String text) { if (column + text.length() > maxLength) { newLine(); + // If the text starts with a space, then eat the space since it isn't needed to separate words now. + if (text.startsWith(" ")) { + builder.append(text, 1, text.length()); + column += text.length() - 1; + return this; + } } builder.append(text); diff --git a/smithy-cli/src/main/java/software/amazon/smithy/cli/commands/BuildOptions.java b/smithy-cli/src/main/java/software/amazon/smithy/cli/commands/BuildOptions.java index 2cf08d72c4c..35f88ec9c7c 100644 --- a/smithy-cli/src/main/java/software/amazon/smithy/cli/commands/BuildOptions.java +++ b/smithy-cli/src/main/java/software/amazon/smithy/cli/commands/BuildOptions.java @@ -15,7 +15,11 @@ package software.amazon.smithy.cli.commands; +import java.nio.file.Path; +import java.nio.file.Paths; import java.util.function.Consumer; +import software.amazon.smithy.build.SmithyBuild; +import software.amazon.smithy.build.model.SmithyBuildConfig; import software.amazon.smithy.cli.ArgumentReceiver; import software.amazon.smithy.cli.HelpPrinter; @@ -70,4 +74,21 @@ String output() { void noPositionalArguments(boolean noPositionalArguments) { this.noPositionalArguments = noPositionalArguments; } + + /** + * Resolves the correct build directory by looking at the --output argument, outputDirectory config setting, + * and finally the default build directory. + * + * @param config Config to check. + * @return Returns the resolved build directory. + */ + Path resolveOutput(SmithyBuildConfig config) { + if (output != null) { + return Paths.get(output); + } else { + return config.getOutputDirectory() + .map(Paths::get) + .orElseGet(SmithyBuild::getDefaultOutputDirectory); + } + } } diff --git a/smithy-cli/src/main/java/software/amazon/smithy/cli/commands/ClasspathAction.java b/smithy-cli/src/main/java/software/amazon/smithy/cli/commands/ClasspathAction.java index a210761745e..75bf97a0ff7 100644 --- a/smithy-cli/src/main/java/software/amazon/smithy/cli/commands/ClasspathAction.java +++ b/smithy-cli/src/main/java/software/amazon/smithy/cli/commands/ClasspathAction.java @@ -17,14 +17,12 @@ import java.io.File; import java.nio.file.Path; -import java.nio.file.Paths; import java.util.ArrayList; import java.util.Collections; import java.util.List; import java.util.Set; import java.util.function.Consumer; import java.util.logging.Logger; -import software.amazon.smithy.build.SmithyBuild; import software.amazon.smithy.build.model.MavenConfig; import software.amazon.smithy.build.model.MavenRepository; import software.amazon.smithy.build.model.SmithyBuildConfig; @@ -134,7 +132,9 @@ private List resolveDependencies( DependencyResolver baseResolver = dependencyResolverFactory.create(smithyBuildConfig, env); long lastModified = smithyBuildConfig.getLastModifiedInMillis(); DependencyResolver delegate = new FilterCliVersionResolver(SmithyCli.getVersion(), baseResolver); - DependencyResolver resolver = new FileCacheResolver(getCacheFile(buildOptions), lastModified, delegate); + DependencyResolver resolver = new FileCacheResolver(getCacheFile(buildOptions, smithyBuildConfig), + lastModified, + delegate); addConfiguredMavenRepos(smithyBuildConfig, resolver); maven.getDependencies().forEach(resolver::addDependency); List artifacts = resolver.resolve(); @@ -171,9 +171,7 @@ private static void addConfiguredMavenRepos(SmithyBuildConfig config, Dependency } } - private File getCacheFile(BuildOptions buildOptions) { - String output = buildOptions.output(); - Path buildPath = output == null ? SmithyBuild.getDefaultOutputDirectory() : Paths.get(output); - return buildPath.resolve("classpath.json").toFile(); + private File getCacheFile(BuildOptions buildOptions, SmithyBuildConfig config) { + return buildOptions.resolveOutput(config).resolve("classpath.json").toFile(); } } diff --git a/smithy-cli/src/main/java/software/amazon/smithy/cli/commands/DiffCommand.java b/smithy-cli/src/main/java/software/amazon/smithy/cli/commands/DiffCommand.java index 08a73137943..6240ed6b37e 100644 --- a/smithy-cli/src/main/java/software/amazon/smithy/cli/commands/DiffCommand.java +++ b/smithy-cli/src/main/java/software/amazon/smithy/cli/commands/DiffCommand.java @@ -1,5 +1,5 @@ /* - * Copyright 2022 Amazon.com, Inc. or its affiliates. All Rights Reserved. + * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. * * Licensed under the Apache License, Version 2.0 (the "License"). * You may not use this file except in compliance with the License. @@ -15,6 +15,8 @@ package software.amazon.smithy.cli.commands; +import java.nio.file.Files; +import java.nio.file.Path; import java.nio.file.Paths; import java.util.Collections; import java.util.List; @@ -28,15 +30,20 @@ import software.amazon.smithy.cli.ColorTheme; import software.amazon.smithy.cli.Command; import software.amazon.smithy.cli.HelpPrinter; +import software.amazon.smithy.cli.StandardOptions; import software.amazon.smithy.cli.dependencies.DependencyResolver; import software.amazon.smithy.diff.ModelDiff; import software.amazon.smithy.model.Model; import software.amazon.smithy.model.validation.Severity; import software.amazon.smithy.model.validation.ValidatedResult; import software.amazon.smithy.model.validation.ValidationEvent; +import software.amazon.smithy.utils.IoUtils; +import software.amazon.smithy.utils.ListUtils; final class DiffCommand implements Command { + private static final String DIFF_WORKTREE_BRANCH = "__smithy-diff-worktree"; + private static final String DIFF_WORKTREE_PATH = "diff-worktree"; private final String parentCommandName; private final DependencyResolver.Factory dependencyResolverFactory; @@ -88,6 +95,11 @@ private String getDocumentation(ColorFormatter colors) { + "is specified." + ls + ls + + " smithy diff --old /path/old --new /path/new" + + ls + + " smithy diff --mode arbitrary --old /path/old --new /path/new" + + ls + + ls + "`--mode project`:" + ls + "Compares the current state of a project against another project. `--old` is required and points " @@ -97,8 +109,26 @@ private String getDocumentation(ColorFormatter colors) { + "loaded using any dependencies defined by the current project. If the `--old` argument points to " + "a directory that contains a `smithy-build.json` file, any `imports` or `sources` defined in that " + "config file will be used when loading the old model, though the dependencies of the old model " - + "are ignored."; - + + "are ignored." + + ls + + ls + + " smithy diff --mode project --old /path/old" + + ls + + ls + + "`--mode git`:" + + ls + + "Compares the current state of a Smithy project to another commit in the current git repo. This " + + "command must be run from within a git repo. The `--old` argument can be provided to specify a " + + "specific revision to compare against. If `--old` is not provided, the commit defaults to `HEAD` " + + "(the last commit on the current branch). This mode is a wrapper around `--mode project`, so its " + + "restrictions apply." + + ls + + ls + + " smithy diff --mode git" + + ls + + " smithy diff --mode git --old main" + + ls + + " smithy diff --mode git --old HEAD~2"; return StyleHelper.markdownLiterals(content, colors); } @@ -134,12 +164,14 @@ public Consumer testParameter(String name) { @Override public void registerHelp(HelpPrinter printer) { + printer.param("--mode", null, "DIFF_MODE", + "The diff mode to use: 'arbitrary' (the default mode), 'project', 'git'."); printer.param("--old", null, "OLD_MODEL", - "Path to an old Smithy model file or directory that contains model files."); + "Path to an old Smithy model file or directory that contains model files. When using " + + "git mode, this argument refers to a Git commit or branch."); printer.param("--new", null, "NEW_MODEL", - "Path to the new Smithy model file or directory that contains model files."); - printer.param("--mode", null, "DIFF_MODE", - "The diff mode to use: 'arbitrary' (default mode), 'project'."); + "Path to the new Smithy model file or directory that contains model files. This argument " + + "is not allowed in project or git mode."); } } @@ -212,10 +244,58 @@ int diff(SmithyBuildConfig config, Arguments arguments, Options options, Env env runDiff(modelBuilder, env, oldModel, newModel); return 0; } + }, + + GIT { + @Override + int diff(SmithyBuildConfig config, Arguments arguments, Options options, Env env) { + // Note: newModel is validated in PROJECT. Old model defaults to HEAD of current branch. + if (options.oldModel == null) { + options.oldModel = "HEAD"; + } + + if (!arguments.getReceiver(StandardOptions.class).quiet()) { + env.colors().println(env.stderr(), "Comparing current project to git " + options.oldModel, + ColorTheme.MUTED); + } + + // Setup a worktree if one isn't present. + Path outputRoot = arguments.getReceiver(BuildOptions.class).resolveOutput(config); + Path worktreePath = outputRoot.resolve(DIFF_WORKTREE_PATH); + Path root = Paths.get("."); + + // Determine the SHA of the given --old branch in the root git directory. + String sha = getSha(root, options.oldModel); + + if (!Files.isDirectory(worktreePath)) { + // First, prune old work trees in case smithy clean was run and left a prunable diff worktree. + exec(ListUtils.of("git", "worktree", "prune"), root, "Error pruning worktrees"); + // Now create the worktree using a dedicated branch. The branch allows other worktrees to checkout + // the same branch or SHA without conflicting. + exec(ListUtils.of("git", "worktree", "add", "--quiet", "--force", "-B", DIFF_WORKTREE_BRANCH, + worktreePath.toString(), sha), + root, "Unable to create git worktree"); + } else { + // Checkout the right commit in the worktree. + exec(ListUtils.of("git", "reset", "--quiet", "--hard", sha), + worktreePath, "Unable to checkout " + options.oldModel + " in git worktree"); + } + + // Now run a project mode build using the worktree. + options.diffMode = PROJECT; + options.oldModel = worktreePath.toString(); + return PROJECT.diff(config, arguments, options, env); + } + + private String getSha(Path root, String commitish) { + // Determine the SHA of the given --old branch in the root git directory. + List args = ListUtils.of("git", "rev-parse", commitish); + return exec(args, root, "Invalid git revision '" + commitish + "'").trim(); + } }; // Create a ModelBuilder template to load the old, then new, then diff both. - protected ModelBuilder createModelBuilder(SmithyBuildConfig config, Arguments arguments, Env env) { + protected final ModelBuilder createModelBuilder(SmithyBuildConfig config, Arguments arguments, Env env) { return new ModelBuilder() .config(config) .arguments(arguments) @@ -227,7 +307,7 @@ protected ModelBuilder createModelBuilder(SmithyBuildConfig config, Arguments ar } // Creating a new model is the same for each diff mode. - protected Model createNewModel(ModelBuilder builder, List models, SmithyBuildConfig config) { + protected final Model createNewModel(ModelBuilder builder, List models, SmithyBuildConfig config) { return builder .models(models) .titleLabel("NEW", ColorTheme.DIFF_EVENT_TITLE) @@ -236,7 +316,7 @@ protected Model createNewModel(ModelBuilder builder, List models, Smithy } // Running the diff is the same for each diff mode. - protected void runDiff(ModelBuilder builder, Env env, Model oldModel, Model newModel) { + protected final void runDiff(ModelBuilder builder, Env env, Model oldModel, Model newModel) { ClassLoader classLoader = env.classLoader(); List events = ModelDiff.compare(classLoader, oldModel, newModel); builder @@ -248,4 +328,13 @@ protected void runDiff(ModelBuilder builder, Env env, Model oldModel, Model newM abstract int diff(SmithyBuildConfig config, Arguments arguments, Options options, Env env); } + + private static String exec(List args, Path root, String errorPrefix) { + StringBuilder output = new StringBuilder(); + int code = IoUtils.runCommand(args, root, output, Collections.emptyMap()); + if (code != 0) { + throw new CliError(errorPrefix + ": " + output); + } + return output.toString(); + } } diff --git a/smithy-cli/src/test/java/software/amazon/smithy/cli/HelpPrinterTest.java b/smithy-cli/src/test/java/software/amazon/smithy/cli/HelpPrinterTest.java index 6ce2b21a6cd..39be4515d54 100644 --- a/smithy-cli/src/test/java/software/amazon/smithy/cli/HelpPrinterTest.java +++ b/smithy-cli/src/test/java/software/amazon/smithy/cli/HelpPrinterTest.java @@ -2,6 +2,7 @@ import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.containsString; +import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.startsWith; import org.junit.jupiter.api.Test; @@ -173,7 +174,7 @@ public void includesWrappedSummaryAndDocumentation() { + "Goodbye18 Goodbye19 Goodbye20 Goodbye21."); help.print(AnsiColorFormatter.NO_COLOR, printer); - assertThat(printer.toString(), startsWith( + assertThat(printer.toString().trim(), equalTo( "Usage: foo [--foo | -f] \n" + "\n" + "Hello1 Hello2 Hello3 Hello4 Hello5 Hello6 Hello7 Hello8 Hello9 Hello10 Hello11 Hello12 Hello13 \n" @@ -184,8 +185,7 @@ public void includesWrappedSummaryAndDocumentation() { + " The foo value\n" + " \n" + "Goodbye1 Goodbye2 Goodbye3 Goodbye4 Goodbye5 Goodbye6 Goodbye7 Goodbye8 Goodbye9 Goodbye10 Goodbye11\n" - + " Goodbye12 Goodbye13 Goodbye14 Goodbye15 Goodbye16 Goodbye17 Goodbye18 Goodbye19 Goodbye20 \n" - + "Goodbye21.")); + + "Goodbye12 Goodbye13 Goodbye14 Goodbye15 Goodbye16 Goodbye17 Goodbye18 Goodbye19 Goodbye20 Goodbye21.")); } @Test diff --git a/smithy-cli/src/test/java/software/amazon/smithy/cli/commands/BuildOptionsTest.java b/smithy-cli/src/test/java/software/amazon/smithy/cli/commands/BuildOptionsTest.java new file mode 100644 index 00000000000..7bb4c04e55e --- /dev/null +++ b/smithy-cli/src/test/java/software/amazon/smithy/cli/commands/BuildOptionsTest.java @@ -0,0 +1,36 @@ +package software.amazon.smithy.cli.commands; + +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.equalTo; + +import java.nio.file.Paths; +import org.junit.jupiter.api.Test; +import software.amazon.smithy.build.SmithyBuild; +import software.amazon.smithy.build.model.SmithyBuildConfig; + +public class BuildOptionsTest { + @Test + public void usesExplicitOutputArgument() { + BuildOptions options = new BuildOptions(); + options.testParameter("--output").accept("test"); + SmithyBuildConfig config = SmithyBuildConfig.builder().version("1").outputDirectory("foo").build(); + + assertThat(options.resolveOutput(config), equalTo(Paths.get("test"))); + } + + @Test + public void usesConfigOutputDirectory() { + BuildOptions options = new BuildOptions(); + SmithyBuildConfig config = SmithyBuildConfig.builder().version("1").outputDirectory("foo").build(); + + assertThat(options.resolveOutput(config), equalTo(Paths.get("foo"))); + } + + @Test + public void usesDefaultBuildDirectory() { + BuildOptions options = new BuildOptions(); + SmithyBuildConfig config = SmithyBuildConfig.builder().version("1").build(); + + assertThat(options.resolveOutput(config), equalTo(SmithyBuild.getDefaultOutputDirectory())); + } +}