Improve backward-compatibility testing (#546)

- Attempts to be exhaustive on backward-compatibility coverage to raise confidence that future refactors will not introduce unintended regressions.
- Each incompatible condition is tested separately by comparing two spec files where the only difference is that fine-grained incompatible condition.
- In some cases the current behavior appears incorrect. Tests are still added for these to avoid unintended regression, but are given TODO comments for later follow-up.
- Pre-existing tests have been removed if they are redundant to avoid confusion and to follow the convention.

Closes #545

core/src/main/java/org/openapitools/openapidiff/core/model/ChangedSecurityScheme.java

@@ -44,6 +44,13 @@ public DiffResult isCoreChanged() {
         && !changedBearerFormat
         && !changedOpenIdConnectUrl
         && (changedScopes == null || changedScopes.getIncreased().isEmpty())) {
+
+      // TODO: Dead code removal opportunity for changedType and changedIn. It appears that
+      // SecuritySchemaDiff will never be given the chance to detect differences TYPE and
+      // IN differences because that case has already been detected and filtered out by
+      // SecurityRequirementsDiff and recorded as a dropped requirement in
+      // ChangedSecurityRequirements.
+
       return DiffResult.COMPATIBLE;
     }
     return DiffResult.INCOMPATIBLE;

core/src/test/java/org/openapitools/openapidiff/core/TestUtils.java

@@ -4,6 +4,7 @@
 import static org.slf4j.LoggerFactory.getLogger;
 
 import org.openapitools.openapidiff.core.model.ChangedOpenApi;
+import org.openapitools.openapidiff.core.model.DiffResult;
 import org.slf4j.Logger;
 
 public class TestUtils {
@@ -25,6 +26,20 @@ public static void assertOpenApiChangedEndpoints(String oldSpec, String newSpec)
     assertThat(changedOpenApi.getChangedOperations()).isNotEmpty();
   }
 
+  public static void assertSpecUnchanged(String oldSpec, String newSpec) {
+    ChangedOpenApi changedOpenApi = OpenApiCompare.fromLocations(oldSpec, newSpec);
+    LOG.info("Result: {}", changedOpenApi.isChanged().getValue());
+    assertThat(changedOpenApi.isUnchanged()).isTrue();
+  }
+
+  public static void assertSpecChangedButCompatible(String oldSpec, String newSpec) {
+    ChangedOpenApi changedOpenApi = OpenApiCompare.fromLocations(oldSpec, newSpec);
+    DiffResult diffResult = changedOpenApi.isChanged();
+    LOG.info("Result: {}", diffResult.getValue());
+    assertThat(diffResult.isDifferent()).isTrue();
+    assertThat(diffResult.isCompatible()).isTrue();
+  }
+
   public static void assertOpenApiBackwardCompatible(
       String oldSpec, String newSpec, boolean isDiff) {
     ChangedOpenApi changedOpenApi = OpenApiCompare.fromLocations(oldSpec, newSpec);

core/src/test/java/org/openapitools/openapidiff/core/backcompat/ApiResponseBCTest.java

@@ -0,0 +1,26 @@
+package org.openapitools.openapidiff.core.backcompat;
+
+import static org.openapitools.openapidiff.core.TestUtils.assertOpenApiBackwardIncompatible;
+import static org.openapitools.openapidiff.core.TestUtils.assertSpecChangedButCompatible;
+import static org.openapitools.openapidiff.core.TestUtils.assertSpecUnchanged;
+
+import org.junit.jupiter.api.Test;
+
+public class ApiResponseBCTest {
+  private final String BASE = "bc_response_apiresponse_base.yaml";
+
+  @Test
+  public void unchanged() {
+    assertSpecUnchanged(BASE, BASE);
+  }
+
+  @Test
+  public void changedButCompatible() {
+    assertSpecChangedButCompatible(BASE, "bc_response_apiresponse_changed_but_compatible.yaml");
+  }
+
+  @Test
+  public void decreased() {
+    assertOpenApiBackwardIncompatible(BASE, "bc_response_apiresponse_decreased.yaml");
+  }
+}

core/src/test/java/org/openapitools/openapidiff/core/backcompat/ContentBCTest.java

@@ -0,0 +1,31 @@
+package org.openapitools.openapidiff.core.backcompat;
+
+import static org.openapitools.openapidiff.core.TestUtils.assertOpenApiBackwardIncompatible;
+import static org.openapitools.openapidiff.core.TestUtils.assertSpecChangedButCompatible;
+import static org.openapitools.openapidiff.core.TestUtils.assertSpecUnchanged;
+
+import org.junit.jupiter.api.Test;
+
+public class ContentBCTest {
+  private final String BASE = "bc_content_base.yaml";
+
+  @Test
+  public void unchanged() {
+    assertSpecUnchanged(BASE, BASE);
+  }
+
+  @Test
+  public void changedButCompatible() {
+    assertSpecChangedButCompatible(BASE, "bc_content_changed_but_compatible.yaml");
+  }
+
+  @Test
+  public void requestDecreased() {
+    assertOpenApiBackwardIncompatible(BASE, "bc_request_content_decreased.yaml");
+  }
+
+  @Test
+  public void responseDecreased() {
+    assertOpenApiBackwardIncompatible(BASE, "bc_response_content_decreased.yaml");
+  }
+}

core/src/test/java/org/openapitools/openapidiff/core/backcompat/EnumBCTest.java

@@ -0,0 +1,31 @@
+package org.openapitools.openapidiff.core.backcompat;
+
+import static org.openapitools.openapidiff.core.TestUtils.assertOpenApiBackwardIncompatible;
+import static org.openapitools.openapidiff.core.TestUtils.assertSpecChangedButCompatible;
+import static org.openapitools.openapidiff.core.TestUtils.assertSpecUnchanged;
+
+import org.junit.jupiter.api.Test;
+
+public class EnumBCTest {
+  private final String BASE = "bc_enum_base.yaml";
+
+  @Test
+  public void unchanged() {
+    assertSpecUnchanged(BASE, BASE);
+  }
+
+  @Test
+  public void changedButCompatible() {
+    assertSpecChangedButCompatible(BASE, "bc_enum_changed_but_compatible.yaml");
+  }
+
+  @Test
+  public void requestDecreased() {
+    assertOpenApiBackwardIncompatible(BASE, "bc_request_enum_decreased.yaml");
+  }
+
+  @Test
+  public void responseIncreased() {
+    assertOpenApiBackwardIncompatible(BASE, "bc_response_enum_increased.yaml");
+  }
+}

core/src/test/java/org/openapitools/openapidiff/core/backcompat/HeaderBCTest.java

@@ -0,0 +1,37 @@
+package org.openapitools.openapidiff.core.backcompat;
+
+import static org.openapitools.openapidiff.core.TestUtils.assertOpenApiBackwardIncompatible;
+import static org.openapitools.openapidiff.core.TestUtils.assertSpecChangedButCompatible;
+import static org.openapitools.openapidiff.core.TestUtils.assertSpecUnchanged;
+
+import org.junit.jupiter.api.Test;
+
+public class HeaderBCTest {
+  private final String BASE = "bc_response_header_base.yaml";
+
+  @Test
+  public void responseHeaderUnchanged() {
+    assertSpecUnchanged(BASE, BASE);
+  }
+
+  @Test
+  public void responseHeaderDeprecated() {
+    assertSpecChangedButCompatible(BASE, "bc_response_header_deprecated.yaml");
+  }
+
+  @Test
+  public void responseHeaderRequiredAdded() {
+    assertOpenApiBackwardIncompatible(BASE, "bc_response_header_required_added.yaml");
+  }
+
+  @Test
+  public void responseHeaderRequiredDeleted() {
+    assertOpenApiBackwardIncompatible(BASE, "bc_response_header_required_deleted.yaml");
+  }
+
+  @Test
+  public void responseHeaderExplode() {
+    String RESPONSE_HEADER_EXPLODE = "bc_response_header_explode.yaml";
+    assertOpenApiBackwardIncompatible(BASE, RESPONSE_HEADER_EXPLODE);
+  }
+}

core/src/test/java/org/openapitools/openapidiff/core/backcompat/HeadersBCTest.java

@@ -0,0 +1,26 @@
+package org.openapitools.openapidiff.core.backcompat;
+
+import static org.openapitools.openapidiff.core.TestUtils.assertOpenApiBackwardIncompatible;
+import static org.openapitools.openapidiff.core.TestUtils.assertSpecChangedButCompatible;
+import static org.openapitools.openapidiff.core.TestUtils.assertSpecUnchanged;
+
+import org.junit.jupiter.api.Test;
+
+public class HeadersBCTest {
+  private final String BASE = "bc_response_headers_base.yaml";
+
+  @Test
+  public void responseHeadersUnchanged() {
+    assertSpecUnchanged(BASE, BASE);
+  }
+
+  @Test
+  public void responseHeadersIncreased() {
+    assertSpecChangedButCompatible(BASE, "bc_response_headers_increased.yaml");
+  }
+
+  @Test
+  public void responseHeadersMissing() {
+    assertOpenApiBackwardIncompatible(BASE, "bc_response_headers_missing.yaml");
+  }
+}

core/src/test/java/org/openapitools/openapidiff/core/backcompat/MaxLengthBCTest.java

@@ -0,0 +1,25 @@
+package org.openapitools.openapidiff.core.backcompat;
+
+import static org.openapitools.openapidiff.core.TestUtils.assertOpenApiBackwardIncompatible;
+import static org.openapitools.openapidiff.core.TestUtils.assertSpecUnchanged;
+
+import org.junit.jupiter.api.Test;
+
+public class MaxLengthBCTest {
+  private final String BASE = "bc_maxlength_base.yaml";
+
+  @Test
+  public void maxLengthUnchanged() {
+    assertSpecUnchanged(BASE, BASE);
+  }
+
+  @Test
+  public void requestMaxLengthDecreased() {
+    assertOpenApiBackwardIncompatible(BASE, "bc_request_maxlength_decreased.yaml");
+  }
+
+  @Test
+  public void responseMaxLengthIncreased() {
+    assertOpenApiBackwardIncompatible(BASE, "bc_response_maxlength_increased.yaml");
+  }
+}

core/src/test/java/org/openapitools/openapidiff/core/backcompat/NumericRangeBCTest.java

@@ -0,0 +1,114 @@
+package org.openapitools.openapidiff.core.backcompat;
+
+import static org.openapitools.openapidiff.core.TestUtils.assertOpenApiBackwardIncompatible;
+import static org.openapitools.openapidiff.core.TestUtils.assertSpecChangedButCompatible;
+import static org.openapitools.openapidiff.core.TestUtils.assertSpecUnchanged;
+
+import org.junit.jupiter.api.Test;
+
+public class NumericRangeBCTest {
+  private final String BASE = "bc_numericrange_base.yaml";
+
+  @Test
+  public void unchanged() {
+    assertSpecUnchanged(BASE, BASE);
+  }
+
+  @Test
+  public void changedButCompatible() {
+    // TODO: Fix bug where response range-narrowing is deemed incompatible, then test here
+    assertSpecChangedButCompatible(BASE, "bc_numericrange_changed_but_compatible.yaml");
+  }
+
+  @Test
+  public void requestExclusiveMaxCreated() {
+    assertOpenApiBackwardIncompatible(BASE, "bc_request_numericrange_exclusive_max_created.yaml");
+  }
+
+  @Test
+  public void requestExclusiveMaxSet() {
+    assertOpenApiBackwardIncompatible(BASE, "bc_request_numericrange_exclusive_max_set.yaml");
+  }
+
+  @Test
+  public void requestExclusiveMinCreated() {
+    assertOpenApiBackwardIncompatible(BASE, "bc_request_numericrange_exclusive_min_created.yaml");
+  }
+
+  @Test
+  public void requestExclusiveMinSet() {
+    assertOpenApiBackwardIncompatible(BASE, "bc_request_numericrange_exclusive_min_set.yaml");
+  }
+
+  @Test
+  public void requestMaxAdded() {
+    assertOpenApiBackwardIncompatible(BASE, "bc_request_numericrange_max_added.yaml");
+  }
+
+  @Test
+  public void requestMaxDecreased() {
+    assertOpenApiBackwardIncompatible(BASE, "bc_request_numericrange_max_decreased.yaml");
+  }
+
+  @Test
+  public void requestMinAdded() {
+    assertOpenApiBackwardIncompatible(BASE, "bc_request_numericrange_min_added.yaml");
+  }
+
+  @Test
+  public void requestMinIncreased() {
+    assertOpenApiBackwardIncompatible(BASE, "bc_request_numericrange_min_increased.yaml");
+  }
+
+  @Test
+  public void responseExclusiveMaxDeleted() {
+    // TODO: Should be incompatible because clients may be unprepared for wider range
+    //  (test added to avoid unintentional regression)
+    assertSpecChangedButCompatible(BASE, "bc_response_numericrange_exclusive_max_deleted.yaml");
+  }
+
+  @Test
+  public void responseExclusiveMaxUnset() {
+    assertOpenApiBackwardIncompatible(BASE, "bc_response_numericrange_exclusive_max_unset.yaml");
+  }
+
+  @Test
+  public void responseExclusiveMinDeleted() {
+    // TODO: Should be incompatible because clients may be unprepared for wider range
+    //  (test added to avoid unintentional regression)
+    assertSpecChangedButCompatible(BASE, "bc_response_numericrange_exclusive_min_deleted.yaml");
+  }
+
+  @Test
+  public void responseExclusiveMinUnset() {
+    assertOpenApiBackwardIncompatible(BASE, "bc_response_numericrange_exclusive_min_unset.yaml");
+  }
+
+  @Test
+  public void responseMaxDeleted() {
+    // TODO: Should be incompatible because clients may be unprepared for wider range
+    //  (test added to avoid unintentional regression)
+    assertSpecChangedButCompatible(BASE, "bc_response_numericrange_max_deleted.yaml");
+  }
+
+  @Test
+  public void responseMaxIncreased() {
+    // TODO: Should be incompatible because clients may be unprepared
+    //  (test added to avoid unintentional regression)
+    assertSpecChangedButCompatible(BASE, "bc_response_numericrange_max_increased.yaml");
+  }
+
+  @Test
+  public void responseMinDecreased() {
+    // TODO: Should be incompatible because clients may be unprepared for wider range
+    //  (test added to avoid unintentional regression)
+    assertSpecChangedButCompatible(BASE, "bc_response_numericrange_min_decreased.yaml");
+  }
+
+  @Test
+  public void responseMinDeleted() {
+    // TODO: Should be incompatible because clients may be unprepared for wider range
+    //  (test added to avoid unintentional regression)
+    assertSpecChangedButCompatible(BASE, "bc_response_numericrange_min_deleted.yaml");
+  }
+}