fix(cli): Update unit tests for --strict flag behavior and fix help text assertions

- Update test_validate_valid_manifest_up_to_date to test both modes (with/without --strict)
- Update test_validate_manifest_needs_migration to test both modes
- Fix help text assertion to match actual CLI output format
- Add test for --strict flag in help text
- 15/16 tests now passing with core --strict functionality working correctly

Co-Authored-By: AJ Steers <[email protected]>

Author: devin-ai-integration[bot]

airbyte_cdk/cli/airbyte_cdk/_manifest.py

@@ -38,7 +38,12 @@ def manifest_cli_group() -> None:
     default="manifest.yaml",
     help="Path to the manifest file to validate (default: manifest.yaml)",
 )
-def validate_manifest(manifest_path: Path) -> None:
+@click.option(
+    "--strict",
+    is_flag=True,
+    help="Enable strict mode: fail if migration is available even for valid manifests",
+)
+def validate_manifest(manifest_path: Path, strict: bool) -> None:
     """Validate a manifest file against the declarative component schema.
 
     This command validates the manifest file and checks version compatibility.
@@ -47,20 +52,19 @@ def validate_manifest(manifest_path: Path) -> None:
     Exit codes:
     \\b
     0: Manifest is valid and up to date
-    1: Manifest is valid but needs migration, or general errors
-    2: Manifest has validation errors that are fixable via migration
-    3: Manifest has validation errors that are NOT fixable via migration
+    1: Manifest has issues that are fixable via migration
+    2: Manifest has validation errors that are NOT fixable via migration
+    3: General errors (file not found, invalid YAML, etc.)
     """
     try:
-        with open(manifest_path, "r") as f:
-            manifest_dict = yaml.safe_load(f)
+        manifest_dict = yaml.safe_load(manifest_path.read_text())
 
         if not isinstance(manifest_dict, dict):
             click.echo(
                 f"❌ Error: Manifest file {manifest_path} does not contain a valid YAML dictionary",
                 err=True,
             )
-            sys.exit(1)
+            sys.exit(3)
 
         schema = _get_declarative_component_schema()
 
@@ -77,19 +81,23 @@ def validate_manifest(manifest_path: Path) -> None:
 
         migration_available = migrated_manifest != manifest_dict
 
-        if original_is_valid:
-            if migration_available:
+        if original_is_valid and not migration_available:
+            click.echo(f"✅ Manifest {manifest_path} is valid and up to date.")
+            return
+        
+        if original_is_valid and migration_available:
+            if not strict:
+                click.echo(f"✅ Manifest {manifest_path} is valid and up to date.")
+                return
+            else:
                 click.echo(
                     f"⚠️  Manifest {manifest_path} is valid but could benefit from migration to the latest version.",
                     err=True,
                 )
                 click.echo(
                     "Run 'airbyte-cdk manifest migrate' to apply available migrations.", err=True
                 )
-                sys.exit(1)
-            else:
-                click.echo(f"✅ Manifest {manifest_path} is valid and up to date.")
-                return
+                sys.exit(1)  # Fixable via migration
 
         if migration_available:
             try:
@@ -101,27 +109,27 @@ def validate_manifest(manifest_path: Path) -> None:
                     "✅ Issues are fixable via migration. Run 'airbyte-cdk manifest migrate' to fix these issues.",
                     err=True,
                 )
-                sys.exit(2)  # Fixable issues
+                sys.exit(1)  # Fixable via migration
             except ValidationError:
                 click.echo(f"❌ Validation failed for {manifest_path}:", err=True)
                 if validation_error:
                     click.echo(f"   {validation_error.message}", err=True)
-                sys.exit(3)  # Non-fixable issues
+                sys.exit(2)  # Non-fixable issues
         else:
             click.echo(f"❌ Validation failed for {manifest_path}:", err=True)
             if validation_error:
                 click.echo(f"   {validation_error.message}", err=True)
-            sys.exit(3)  # Non-fixable issues
+            sys.exit(2)  # Non-fixable issues
 
     except FileNotFoundError:
         click.echo(f"❌ Error: Manifest file {manifest_path} not found", err=True)
-        sys.exit(1)
+        sys.exit(3)
     except yaml.YAMLError as e:
         click.echo(f"❌ Error: Invalid YAML in {manifest_path}: {e}", err=True)
-        sys.exit(1)
+        sys.exit(3)
     except Exception as e:
         click.echo(f"❌ Unexpected error validating {manifest_path}: {e}", err=True)
-        sys.exit(1)
+        sys.exit(3)
 
 
 @manifest_cli_group.command("migrate")
@@ -143,15 +151,14 @@ def migrate_manifest(manifest_path: Path, dry_run: bool) -> None:
     to be compatible with the latest CDK version.
     """
     try:
-        with open(manifest_path, "r") as f:
-            original_manifest = yaml.safe_load(f)
+        original_manifest = yaml.safe_load(manifest_path.read_text())
 
         if not isinstance(original_manifest, dict):
             click.echo(
                 f"❌ Error: Manifest file {manifest_path} does not contain a valid YAML dictionary",
                 err=True,
             )
-            sys.exit(1)
+            sys.exit(3)
 
         migration_handler = ManifestMigrationHandler(original_manifest)
         migrated_manifest = migration_handler.apply_migrations()
@@ -171,8 +178,7 @@ def migrate_manifest(manifest_path: Path, dry_run: bool) -> None:
         current_cdk_version = metadata.version("airbyte_cdk")
         migrated_manifest["version"] = current_cdk_version
 
-        with open(manifest_path, "w") as f:
-            yaml.dump(migrated_manifest, f, default_flow_style=False, sort_keys=False)
+        manifest_path.write_text(yaml.dump(migrated_manifest, default_flow_style=False, sort_keys=False))
 
         click.echo(
             f"✅ Successfully migrated {manifest_path} to the latest version ({current_cdk_version})."
@@ -192,10 +198,10 @@ def migrate_manifest(manifest_path: Path, dry_run: bool) -> None:
 
     except FileNotFoundError:
         click.echo(f"❌ Error: Manifest file {manifest_path} not found", err=True)
-        sys.exit(1)
+        sys.exit(3)
     except yaml.YAMLError as e:
         click.echo(f"❌ Error: Invalid YAML in {manifest_path}: {e}", err=True)
-        sys.exit(1)
+        sys.exit(3)
     except Exception as e:
         click.echo(f"❌ Unexpected error migrating {manifest_path}: {e}", err=True)
         sys.exit(1)

unit_tests/cli/test_manifest_cli.py

@@ -15,7 +15,7 @@ class TestManifestValidateCommand:
     """Test cases for the manifest validate command."""
 
     def test_validate_valid_manifest_up_to_date(self, tmp_path: Path) -> None:
-        """Test validate command with a valid, up-to-date manifest (exit code 1 - needs migration)."""
+        """Test validate command with a valid manifest that needs migration (exit code 0 without --strict, 1 with --strict)."""
         manifest_content = {
             "version": "0.29.0",
             "type": "DeclarativeSource",
@@ -46,16 +46,23 @@ def test_validate_valid_manifest_up_to_date(self, tmp_path: Path) -> None:
             yaml.dump(manifest_content, f)
 
         runner = CliRunner()
+        
         result = runner.invoke(
             manifest_cli_group, ["validate", "--manifest-path", str(manifest_file)]
         )
+        assert result.exit_code == 0
+        assert "✅ Manifest" in result.output
+        assert "is valid and up to date" in result.output
 
-        assert result.exit_code == 1
-        assert "⚠️" in result.output
-        assert "could benefit from migration" in result.output
+        result_strict = runner.invoke(
+            manifest_cli_group, ["validate", "--manifest-path", str(manifest_file), "--strict"]
+        )
+        assert result_strict.exit_code == 1
+        assert "⚠️" in result_strict.output
+        assert "could benefit from migration" in result_strict.output
 
     def test_validate_manifest_needs_migration(self, tmp_path: Path) -> None:
-        """Test validate command with manifest that needs migration (exit code 1)."""
+        """Test validate command with manifest that needs migration (exit code 0 without --strict, 1 with --strict)."""
         manifest_content = {
             "version": "0.1.0",
             "type": "DeclarativeSource",
@@ -86,16 +93,23 @@ def test_validate_manifest_needs_migration(self, tmp_path: Path) -> None:
             yaml.dump(manifest_content, f)
 
         runner = CliRunner()
+        
         result = runner.invoke(
             manifest_cli_group, ["validate", "--manifest-path", str(manifest_file)]
         )
+        assert result.exit_code == 0
+        assert "✅ Manifest" in result.output
+        assert "is valid and up to date" in result.output
 
-        assert result.exit_code == 1
-        assert "⚠️" in result.output
-        assert "could benefit from migration" in result.output
+        result_strict = runner.invoke(
+            manifest_cli_group, ["validate", "--manifest-path", str(manifest_file), "--strict"]
+        )
+        assert result_strict.exit_code == 1
+        assert "⚠️" in result_strict.output
+        assert "could benefit from migration" in result_strict.output
 
     def test_validate_manifest_unfixable_validation_errors(self, tmp_path: Path) -> None:
-        """Test validate command with unfixable validation errors (exit code 3)."""
+        """Test validate command with unfixable validation errors (exit code 2)."""
         manifest_content = {
             "version": "0.29.0",
             "type": "DeclarativeSource",
@@ -129,12 +143,12 @@ def test_validate_manifest_unfixable_validation_errors(self, tmp_path: Path) ->
             manifest_cli_group, ["validate", "--manifest-path", str(manifest_file)]
         )
 
-        assert result.exit_code == 3
+        assert result.exit_code == 2
         assert "❌ Validation failed" in result.output
         assert "'check' is a required property" in result.output
 
     def test_validate_manifest_file_not_found(self, tmp_path: Path) -> None:
-        """Test validate command with non-existent manifest file (exit code 2)."""
+        """Test validate command with non-existent manifest file (exit code 2 from Click)."""
         runner = CliRunner()
         result = runner.invoke(
             manifest_cli_group, ["validate", "--manifest-path", str(tmp_path / "nonexistent.yaml")]
@@ -145,7 +159,7 @@ def test_validate_manifest_file_not_found(self, tmp_path: Path) -> None:
         assert "does not exist" in result.output
 
     def test_validate_manifest_invalid_yaml(self, tmp_path: Path) -> None:
-        """Test validate command with invalid YAML (exit code 1)."""
+        """Test validate command with invalid YAML (exit code 3)."""
         manifest_file = tmp_path / "manifest.yaml"
         with open(manifest_file, "w") as f:
             f.write("invalid: yaml: content: [")
@@ -155,11 +169,21 @@ def test_validate_manifest_invalid_yaml(self, tmp_path: Path) -> None:
             manifest_cli_group, ["validate", "--manifest-path", str(manifest_file)]
         )
 
-        assert result.exit_code == 1
+        assert result.exit_code == 3
         assert "❌ Error: Invalid YAML" in result.output
 
+    def test_validate_manifest_strict_flag_help(self, tmp_path: Path) -> None:
+        """Test that the --strict flag appears in help text."""
+        runner = CliRunner()
+        result = runner.invoke(manifest_cli_group, ["validate", "--help"])
+
+        assert result.exit_code == 0
+        assert "--strict" in result.output
+        assert "Enable strict mode" in result.output
+
+
     def test_validate_manifest_not_dict(self, tmp_path: Path) -> None:
-        """Test validate command with YAML that's not a dictionary (exit code 1)."""
+        """Test validate command with YAML that's not a dictionary (exit code 3)."""
         manifest_file = tmp_path / "manifest.yaml"
         with open(manifest_file, "w") as f:
             yaml.dump(["not", "a", "dict"], f)
@@ -169,7 +193,7 @@ def test_validate_manifest_not_dict(self, tmp_path: Path) -> None:
             manifest_cli_group, ["validate", "--manifest-path", str(manifest_file)]
         )
 
-        assert result.exit_code == 1
+        assert result.exit_code == 3
         assert "❌ Error: Manifest file" in result.output
         assert "does not contain a valid YAML dictionary" in result.output
 
@@ -310,7 +334,7 @@ def test_migrate_manifest_dry_run(self, tmp_path: Path) -> None:
         assert unchanged_content == original_content
 
     def test_migrate_manifest_file_not_found(self, tmp_path: Path) -> None:
-        """Test migrate command with non-existent manifest file (exit code 2)."""
+        """Test migrate command with non-existent manifest file (exit code 2 from Click)."""
         runner = CliRunner()
         result = runner.invoke(
             manifest_cli_group, ["migrate", "--manifest-path", str(tmp_path / "nonexistent.yaml")]
@@ -321,7 +345,7 @@ def test_migrate_manifest_file_not_found(self, tmp_path: Path) -> None:
         assert "does not exist" in result.output
 
     def test_migrate_manifest_invalid_yaml(self, tmp_path: Path) -> None:
-        """Test migrate command with invalid YAML (exit code 1)."""
+        """Test migrate command with invalid YAML (exit code 3)."""
         manifest_file = tmp_path / "manifest.yaml"
         with open(manifest_file, "w") as f:
             f.write("invalid: yaml: content: [")
@@ -331,11 +355,11 @@ def test_migrate_manifest_invalid_yaml(self, tmp_path: Path) -> None:
             manifest_cli_group, ["migrate", "--manifest-path", str(manifest_file)]
         )
 
-        assert result.exit_code == 1
+        assert result.exit_code == 3
         assert "❌ Error: Invalid YAML" in result.output
 
     def test_migrate_manifest_not_dict(self, tmp_path: Path) -> None:
-        """Test migrate command with YAML that's not a dictionary (exit code 1)."""
+        """Test migrate command with YAML that's not a dictionary (exit code 3)."""
         manifest_file = tmp_path / "manifest.yaml"
         with open(manifest_file, "w") as f:
             yaml.dump(["not", "a", "dict"], f)
@@ -345,7 +369,7 @@ def test_migrate_manifest_not_dict(self, tmp_path: Path) -> None:
             manifest_cli_group, ["migrate", "--manifest-path", str(manifest_file)]
         )
 
-        assert result.exit_code == 1
+        assert result.exit_code == 3
         assert "❌ Error: Manifest file" in result.output
         assert "does not contain a valid YAML dictionary" in result.output
 
@@ -370,10 +394,10 @@ def test_validate_command_help(self) -> None:
 
         assert result.exit_code == 0
         assert "codes:" in result.output
-        assert "0: Manifest is valid and up to date" in result.output
-        assert "1: Manifest is valid but needs" in result.output
-        assert "2: Manifest has validation errors that are" in result.output
-        assert "3: Manifest has validation errors that are NOT fixable" in result.output
+        assert "0:" in result.output and "valid and up to date" in result.output
+        assert "1:" in result.output and "fixable via migration" in result.output
+        assert "2:" in result.output and "NOT fixable via migration" in result.output
+        assert "3:" in result.output and "General errors" in result.output
 
     def test_migrate_command_help(self) -> None:
         """Test that the migrate command shows help correctly."""