[Partial] 168 change the references of the old version to the new ver…

…sion (#247)

* Adds check to bump-recipe that ensures version is provided when needed

* Fixes/updates some of the bump-recipe unit tests

* Fixes remaining bump-recipe unit tests from version option fall-out

* Moves test_bump_recipe.py to use read-only parser when editing commands are not used in assert statements

* test_bump_recipe.py now uses more consistent names across tests.

* Starts work on version update capability

* Temp workaround for linting error

conda_recipe_manager/commands/bump_recipe.py

@@ -6,7 +6,7 @@
 
 import sys
 from pathlib import Path
-from typing import cast
+from typing import Optional, cast
 
 import click
 
@@ -49,30 +49,35 @@ def _update_build_num(recipe_parser: RecipeParser, increment_build_num: bool) ->
         print_err("`/build` key could not be found in the recipe.")
         sys.exit(ExitCode.ILLEGAL_OPERATION)
 
-    # If build key is found, try to get build/number key in case of `build_num` set to false, `build/number` key will be
-    # added and set to zero when `build_num` is set to true, throw error and sys.exit()
-
-    # TODO use contains_value() instead of try catch
-    try:
+    # From the previous check, we know that `/build` exists. If `/build/number` is missing, it'll be added by
+    # a patch-add operation and set to a default value of 0. Otherwise, we attempt to increment the build number, if
+    # requested.
+    if increment_build_num and recipe_parser.contains_value("/build/number"):
         build_number = recipe_parser.get_value("/build/number")
-        if increment_build_num:
-            if not isinstance(build_number, int):
-                print_err("Build number is not an integer.")
-                sys.exit(ExitCode.ILLEGAL_OPERATION)
-
-            _exit_on_failed_patch(
-                recipe_parser,
-                cast(JsonPatchType, {"op": "replace", "path": "/build/number", "value": build_number + 1}),
-            )
-            return
-    except KeyError:
-        if increment_build_num:
-            print_err("`/build/number` key could not be found in the recipe.")
+
+        if not isinstance(build_number, int):
+            print_err("Build number is not an integer.")
             sys.exit(ExitCode.ILLEGAL_OPERATION)
 
+        _exit_on_failed_patch(
+            recipe_parser,
+            cast(JsonPatchType, {"op": "replace", "path": "/build/number", "value": build_number + 1}),
+        )
+        return
+
     _exit_on_failed_patch(recipe_parser, cast(JsonPatchType, {"op": "add", "path": "/build/number", "value": 0}))
 
 
+def _update_version(recipe_parser: RecipeParser, target_version: str) -> None:  # pylint: disable=unused-argument
+    """
+    Attempts to update the `/package/version` field and/or the commonly used `version` JINJA variable.
+
+    :param recipe_parser: Recipe file to update.
+    :param target_version: Target version to update to.
+    """
+    # TODO branch on `/package/version` being specified without a `version` variable
+
+
 def _update_sha256(recipe_parser: RecipeParser) -> None:
     """
     Attempts to update the SHA-256 hash(s) in the `/source` section of a recipe file, if applicable. Note that this is
@@ -117,16 +122,29 @@ def _update_sha256(recipe_parser: RecipeParser) -> None:
 @click.command(short_help="Bumps a recipe file to a new version.")
 @click.argument("recipe_file_path", type=click.Path(exists=True, path_type=str))
 @click.option(
+    "-b",
     "--build-num",
     is_flag=True,
     help="Bump the build number by 1.",
 )
-def bump_recipe(recipe_file_path: str, build_num: bool) -> None:
+@click.option(
+    "-t",
+    "--target-version",
+    default=None,
+    type=str,
+    help="New project version to target. Required if `--build-num` is NOT specified.",
+)
+def bump_recipe(recipe_file_path: str, build_num: bool, target_version: Optional[str]) -> None:
     """
     Bumps a recipe to a new version.
 
     RECIPE_FILE_PATH: Path to the target recipe file
     """
+
+    if not build_num and target_version is None:
+        print_err("The `--target-version` option must be set if `--build-num` is not specified.")
+        sys.exit(ExitCode.CLICK_USAGE)
+
     try:
         contents_recipe = Path(recipe_file_path).read_text(encoding="utf-8")
     except IOError:
@@ -141,7 +159,13 @@ def bump_recipe(recipe_file_path: str, build_num: bool) -> None:
 
     # Attempt to update fields
     _update_build_num(recipe_parser, build_num)
-    if not build_num:
+
+    # NOTE: We check if `target_version` is specified to perform a "full bump" for type checking reasons. Also note that
+    # the `build_num` flag is invalidated if we are bumping to a new version. The build number must be reset to 0 in
+    # this case.
+    if target_version is not None:
+        # Version must be updated before hash to ensure the correct artifact is hashed.
+        _update_version(recipe_parser, target_version)
         _update_sha256(recipe_parser)
 
     Path(recipe_file_path).write_text(recipe_parser.render(), encoding="utf-8")

tests/commands/test_bump_recipe.py

@@ -2,7 +2,8 @@
 :Description: Tests the `bump-recipe` CLI
 """
 
-from typing import Final, cast
+from pathlib import Path
+from typing import Final, Optional, cast
 from unittest.mock import patch
 
 import pytest
@@ -12,7 +13,7 @@
 from conda_recipe_manager.commands import bump_recipe
 from conda_recipe_manager.commands.utils.types import ExitCode
 from conda_recipe_manager.fetcher.http_artifact_fetcher import HttpArtifactFetcher
-from conda_recipe_manager.parser.recipe_parser import RecipeParser
+from conda_recipe_manager.parser.recipe_reader import RecipeReader
 from tests.file_loading import get_test_path, load_recipe
 from tests.http_mocking import MockHttpStreamResponse
 from tests.smoke_testing import assert_cli_usage
@@ -77,32 +78,35 @@ def test_usage() -> None:
 
 
 @pytest.mark.parametrize(
-    "recipe_file,increment_build_num,expected_recipe_file",
+    "recipe_file,version,expected_recipe_file",
     [
-        ("types-toml.yaml", True, "bump_recipe/types-toml_build_num_1.yaml"),
+        ("types-toml.yaml", None, "bump_recipe/types-toml_build_num_1.yaml"),
         # TODO: Re-enable test when version bumping is supported.
-        # ("types-toml.yaml", False, "bump_recipe/types-toml_version_bump.yaml"),
+        # ("types-toml.yaml", "0.10.8.20240310", "bump_recipe/types-toml_version_bump.yaml"),
         # NOTE: These have no source section, therefore all SHA-256 update attempts (and associated network requests)
         # should be skipped.
-        ("bump_recipe/build_num_1.yaml", True, "bump_recipe/build_num_2.yaml"),
-        ("bump_recipe/build_num_1.yaml", False, "simple-recipe.yaml"),
-        ("bump_recipe/build_num_42.yaml", True, "bump_recipe/build_num_43.yaml"),
-        ("bump_recipe/build_num_42.yaml", False, "simple-recipe.yaml"),
-        ("bump_recipe/build_num_-1.yaml", True, "simple-recipe.yaml"),
-        ("bump_recipe/build_num_-1.yaml", False, "simple-recipe.yaml"),
+        ("bump_recipe/build_num_1.yaml", None, "bump_recipe/build_num_2.yaml"),
+        ("bump_recipe/build_num_1.yaml", "0.10.8.6", "simple-recipe.yaml"),
+        ("bump_recipe/build_num_42.yaml", None, "bump_recipe/build_num_43.yaml"),
+        ("bump_recipe/build_num_42.yaml", "0.10.8.6", "simple-recipe.yaml"),
+        ("bump_recipe/build_num_-1.yaml", None, "simple-recipe.yaml"),
+        ("bump_recipe/build_num_-1.yaml", "0.10.8.6", "simple-recipe.yaml"),
     ],
 )
 def test_bump_recipe_cli(
     fs: FakeFilesystem,
     recipe_file: str,
-    increment_build_num: bool,
+    version: Optional[str],
     expected_recipe_file: str,
 ) -> None:
     """
     Test that the recipe file is successfully updated/bumped.
 
     :param fs: `pyfakefs` Fixture used to replace the file system
-    :param request: Pytest fixture request object.
+    :param recipe_file: Target recipe file to update
+    :param version: (Optional) version to bump to. If `None`, this indicates `bump-recipe` should be run in
+        increment-only mode.
+    :param expected_recipe_file: Expected resulting recipe file
     """
     runner = CliRunner()
     fs.add_real_directory(get_test_path(), read_only=False)
@@ -111,41 +115,54 @@ def test_bump_recipe_cli(
     expected_recipe_file_path = get_test_path() / expected_recipe_file
 
     cli_args: Final[list[str]] = (
-        ["--build-num", str(recipe_file_path)] if increment_build_num else [str(recipe_file_path)]
+        ["--build-num", str(recipe_file_path)] if version is None else ["-t", version, str(recipe_file_path)]
     )
 
     with patch("requests.get", new=mock_requests_get):
         result = runner.invoke(bump_recipe.bump_recipe, cli_args)
 
-    parser = load_recipe(recipe_file_path, RecipeParser)
-    expected_parser = load_recipe(expected_recipe_file_path, RecipeParser)
+    # Read the edited file and check it against the expected file.
+    parser = load_recipe(recipe_file_path, RecipeReader)
+    expected_parser = load_recipe(expected_recipe_file_path, RecipeReader)
 
     assert parser == expected_parser
     assert result.exit_code == ExitCode.SUCCESS
 
 
-def test_bump_cli_build_number_key_missing(fs: FakeFilesystem) -> None:
+def test_bump_recipe_exits_if_target_version_missing() -> None:
     """
-    Test that a `build: number:` key is added and set to 0 when it's missing.
+    Ensures that the `--target-version` flag is required when `--build-num` is NOT provided.
+    """
+    runner = CliRunner()
+    result = runner.invoke(bump_recipe.bump_recipe, [str(get_test_path() / "types-toml.yaml")])
+
+    # Ensure the expected error case was hit (as opposed to a different error case).
+    assert result.stdout == "The `--target-version` option must be set if `--build-num` is not specified.\n"
+    assert result.exit_code == ExitCode.CLICK_USAGE
+
+
+def test_bump_recipe_increment_build_number_key_missing(fs: FakeFilesystem) -> None:
+    """
+    Test that a `/build/number` key is added and set to 0 when it's missing.
 
     :param fs: `pyfakefs` Fixture used to replace the file system
     """
     runner = CliRunner()
     fs.add_real_directory(get_test_path(), read_only=False)
 
-    recipe_file_path = get_test_path() / "bump_recipe/no_build_num.yaml"
-    expected_recipe_file_path = get_test_path() / "bump_recipe/build_num_added.yaml"
+    recipe_file_path: Final[Path] = get_test_path() / "bump_recipe/no_build_num.yaml"
+    expected_recipe_file_path: Final[Path] = get_test_path() / "bump_recipe/build_num_added.yaml"
 
-    result = runner.invoke(bump_recipe.bump_recipe, [str(recipe_file_path)])
+    result = runner.invoke(bump_recipe.bump_recipe, ["--build-num", str(recipe_file_path)])
 
-    parser = load_recipe(recipe_file_path, RecipeParser)
-    expected_parser = load_recipe(expected_recipe_file_path, RecipeParser)
+    parser = load_recipe(recipe_file_path, RecipeReader)
+    expected_parser = load_recipe(expected_recipe_file_path, RecipeReader)
 
     assert parser == expected_parser
     assert result.exit_code == ExitCode.SUCCESS
 
 
-def test_bump_cli_build_number_not_int(fs: FakeFilesystem) -> None:
+def test_bump_recipe_increment_build_number_not_int(fs: FakeFilesystem) -> None:
     """
     Test that the command fails gracefully case when the build number is not an integer,
     and we are trying to increment it.
@@ -156,28 +173,32 @@ def test_bump_cli_build_number_not_int(fs: FakeFilesystem) -> None:
     runner = CliRunner()
     fs.add_real_directory(get_test_path(), read_only=False)
 
-    recipe_file_path = get_test_path() / "bump_recipe/non_int_build_num.yaml"
+    recipe_file_path: Final[Path] = get_test_path() / "bump_recipe/non_int_build_num.yaml"
 
     result = runner.invoke(bump_recipe.bump_recipe, ["--build-num", str(recipe_file_path)])
     assert result.exit_code == ExitCode.ILLEGAL_OPERATION
 
 
-def test_bump_cli_increment_build_num_key_not_found(fs: FakeFilesystem) -> None:
+def test_bump_recipe_increment_build_num_key_not_found(fs: FakeFilesystem) -> None:
     """
-    Test that the command fails gracefully when the build number key is missing and we try to increment it's value.
+    Test that the command fixes the recipe file when the `/build/number` key is missing and we try to increment it's
+    value.
 
     :param fs: `pyfakefs` Fixture used to replace the file system
     """
 
     runner = CliRunner()
     fs.add_real_directory(get_test_path(), read_only=False)
 
-    recipe_file_path = get_test_path() / "bump_recipe/no_build_num.yaml"
+    recipe_file_path: Final[Path] = get_test_path() / "bump_recipe/no_build_num.yaml"
     result = runner.invoke(bump_recipe.bump_recipe, ["--build-num", str(recipe_file_path)])
-    assert result.exit_code == ExitCode.ILLEGAL_OPERATION
+    # TODO: Can't compare directly to `simple-recipe.yaml` as the added key `/build/number` is not canonically sorted to
+    # be in the standard position.
+    assert load_recipe(recipe_file_path, RecipeReader).get_value("/build/number") == 0
+    assert result.exit_code == ExitCode.SUCCESS
 
 
-def test_bump_cli_no_build_key_found(fs: FakeFilesystem) -> None:
+def test_bump_recipe_increment_no_build_key_found(fs: FakeFilesystem) -> None:
     """
     Test that the command fails gracefully when the build key is missing and we try to revert build number to zero.
 
@@ -187,6 +208,6 @@ def test_bump_cli_no_build_key_found(fs: FakeFilesystem) -> None:
     runner = CliRunner()
     fs.add_real_directory(get_test_path(), read_only=False)
 
-    recipe_file_path = get_test_path() / "bump_recipe/no_build_key.yaml"
-    result = runner.invoke(bump_recipe.bump_recipe, [str(recipe_file_path)])
+    recipe_file_path: Final[Path] = get_test_path() / "bump_recipe/no_build_key.yaml"
+    result = runner.invoke(bump_recipe.bump_recipe, ["--build-num", str(recipe_file_path)])
     assert result.exit_code == ExitCode.ILLEGAL_OPERATION