Skip to content

Commit

Permalink
Make default diff mode --git
Browse files Browse the repository at this point in the history
Rather than fail when `smithy diff` is called with no arguments, we
will insteadd assume that the caller meant `smithy diff --mode git`
and use `git` mode against HEAD.

So now:

```
smithy diff # mode is --diff
smithy diff --old some-file --new some-file # mode is --arbitary
smithy diff --old some-file # Fails due to --arbitrary missing --new
smithy diff --new some-file # Fails due to --arbitrary missing --old
```
  • Loading branch information
mtdowling committed Jul 13, 2023
1 parent 4456d2a commit 1acf7dc
Show file tree
Hide file tree
Showing 2 changed files with 38 additions and 10 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@ public void doesNotUseImportsOrSourcesWithArbitraryMode() {

@Test
public void requiresOldAndNewForArbitraryMode() {
RunResult result = IntegUtils.run(Paths.get("."), ListUtils.of("diff"));
RunResult result = IntegUtils.run(Paths.get("."), ListUtils.of("diff", "--mode", "arbitrary"));

assertThat("Not 1: output [" + result.getOutput() + ']', result.getExitCode(), is(1));
assertThat(result.getOutput(), containsString("Missing required --old argument"));
Expand Down Expand Up @@ -181,14 +181,18 @@ public void gitDiffAgainstLastCommit() {
}

private RunResult runDiff(Path dir) {
return runDiff(dir, null);
return runDiff(dir, null, true);
}

private RunResult runDiff(Path dir, String oldCommit) {
private RunResult runDiff(Path dir, String oldCommit, boolean explicitMode) {
List<String> args = new ArrayList<>();
args.add("diff");
args.add("--mode");
args.add("git");

if (explicitMode) {
args.add("--mode");
args.add("git");
}

if (oldCommit != null) {
args.add("--old");
args.add(oldCommit);
Expand Down Expand Up @@ -272,14 +276,24 @@ public void gitDiffEnsuresHeadUpdatesAsCommitsAreMade() {
});
}

@Test
public void gitDiffModeUsedByDefault() {
IntegUtils.withProject("simple-config-sources", dir -> {
initRepo(dir);
commitChanges(dir);
RunResult result = runDiff(dir, null, false);
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");
RunResult result = runDiff(dir, "HEAD", true);
assertThat("Not zero: output [" + result.getOutput() + ']', result.getExitCode(), is(0));

// Now make another commit, which updates HEAD and ensures the worktree updates too.
Expand All @@ -289,7 +303,7 @@ public void gitDiffAgainstSpecificCommit() {

// 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");
result = runDiff(dir, "HEAD~1", true);

assertThat("Not 1: output [" + result.getOutput() + ']', result.getExitCode(), is(1));
assertThat(result.getOutput(), containsString("ChangedShapeType"));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ private String getDocumentation(ColorFormatter colors) {
+ "When run within a project directory that contains a `smithy-build.json` config, any dependencies "
+ "defined in the config file are used when loading both the old and new models; however, `imports` "
+ "and `sources` defined in the config file are not used. This is the default mode when no `--mode` "
+ "is specified."
+ "is specified and `--old` or `--new` are provided."
+ ls
+ ls
+ " smithy diff --old /path/old --new /path/new"
Expand Down Expand Up @@ -121,9 +121,12 @@ private String getDocumentation(ColorFormatter colors) {
+ "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."
+ "restrictions apply. This is the default mode when no arguments are provided."
+ ls
+ ls
+ " # Equivalent to `smithy diff --mode git`"
+ " smithy diff"
+ ls
+ " smithy diff --mode git"
+ ls
+ " smithy diff --mode git --old main"
Expand All @@ -133,7 +136,7 @@ private String getDocumentation(ColorFormatter colors) {
}

private static final class Options implements ArgumentReceiver {
private DiffMode diffMode = DiffMode.ARBITRARY;
private DiffMode diffMode = DiffMode.DETECTED;
private String oldModel;
private String newModel;

Expand Down Expand Up @@ -186,6 +189,17 @@ int runWithClassLoader(SmithyBuildConfig config, Arguments arguments, Env env) {
}

private enum DiffMode {
DETECTED {
@Override
int diff(SmithyBuildConfig config, Arguments arguments, Options options, Env env) {
if (options.oldModel != null || options.newModel != null) {
return ARBITRARY.diff(config, arguments, options, env);
} else {
return GIT.diff(config, arguments, options, env);
}
}
},

ARBITRARY {
@Override
int diff(SmithyBuildConfig config, Arguments arguments, Options options, Env env) {
Expand Down

0 comments on commit 1acf7dc

Please sign in to comment.