chore: incorporates changes based on PR feedback

Signed-off-by: Jennifer Power <[email protected]>

tests/e2e/test_e2e_ssp.py

@@ -19,6 +19,13 @@
 
 Notes that this should be the only E2E for auto-syncing since the UX is the same for each model.
 The SSP model is used here as a stand-in for all models because it is the most complex process.
+
+The tests here are based on the following workflow:
+1. Create new SSP
+2. Autosync SSP to create initial Markdown
+3. Run autosync again to check that no changes are pushed
+4. Update the profile with sync-upstreams
+5. Autosync again to check that the changes are pushed
 """
 
 import logging

tests/testutils.py

@@ -400,7 +400,13 @@ def prepare_upstream_repo() -> str:
     """
     Prepare a temporary upstream repo for testing.
 
-    The include the test NIST catalog and a modified profile.
+    Returns:
+        str: Path to the upstream repo
+
+    Notes:
+    This includes the test NIST catalog and a modified profile.
+    It modifies the simplified_nist_profile to simulate upstream
+    changes for testing.
     """
     tmp_dir = pathlib.Path(tempfile.mkdtemp())
     repo: Repo = repo_setup(tmp_dir)

tests/trestlebot/entrypoints/test_sync_upstreams.py

@@ -42,31 +42,47 @@ def valid_args_dict() -> Dict[str, str]:
 test_cat = "simplified_nist_catalog"
 test_cat_path = "catalogs/simplified_nist_catalog/catalog.json"
 test_prof = "simplified_nist_profile"
+test_prof_path = "profiles/simplified_nist_profile/profile.json"
+test_repo_url = "git.test.com/test/repo.git"
 
 
-def test_with_no_sources(valid_args_dict: Dict[str, str], caplog: Any) -> None:
-    """Test with an invalid source argument."""
+def test_sync_upstreams(
+    tmp_repo: Tuple[str, Repo], valid_args_dict: Dict[str, str]
+) -> None:
+    """Test sync upstreams with default settings and valid args."""
+    repo_path, repo = tmp_repo
+    repo.create_remote("origin", url=test_repo_url)
+
     args_dict = valid_args_dict
-    args_dict["sources"] = ""
+    args_dict["working-dir"] = repo_path
 
-    with patch("sys.argv", ["trestlebot", *args_dict_to_list(args_dict)]):
-        with pytest.raises(SystemExit, match="2"):
+    source: str = prepare_upstream_repo()
+
+    args_dict["sources"] = f"{source}@main"
+
+    with patch("git.remote.Remote.push") as mock_push, patch(
+        "sys.argv", ["trestlebot", *args_dict_to_list(args_dict)]
+    ):
+        mock_push.return_value = "Mocked Results"
+        with pytest.raises(SystemExit, match="0"):
             cli_main()
 
-    assert any(
-        record.levelno == logging.ERROR
-        and "Invalid args --sources: Must set at least one source to sync from."
-        in record.message
-        for record in caplog.records
-    )
+    # Verify that the correct files were included
+    commit = next(repo.iter_commits())
+    assert test_cat_path in commit.stats.files
+    assert test_prof_path in commit.stats.files
+    assert len(commit.stats.files) == 2
+
+    # Clean up the source repo
+    clean(source, None)
 
 
 def test_with_include_models(
     tmp_repo: Tuple[str, Repo], valid_args_dict: Dict[str, str]
 ) -> None:
-    """Test with include_models."""
+    """Test sync upstreams with include models flag."""
     repo_path, repo = tmp_repo
-    repo.create_remote("origin", url="git.test.com/test/repo.git")
+    repo.create_remote("origin", url=test_repo_url)
 
     args_dict = valid_args_dict
     args_dict["include-models"] = test_cat
@@ -83,7 +99,7 @@ def test_with_include_models(
         with pytest.raises(SystemExit, match="0"):
             cli_main()
 
-    # Verify the the correct files were included
+    # Verify that the correct files were included
     commit = next(repo.iter_commits())
     assert test_cat_path in commit.stats.files
     assert len(commit.stats.files) == 1
@@ -95,9 +111,9 @@ def test_with_include_models(
 def test_with_exclude_models(
     tmp_repo: Tuple[str, Repo], valid_args_dict: Dict[str, str]
 ) -> None:
-    """Test with exclude models."""
+    """Test sync upstreams with exclude models flag."""
     repo_path, repo = tmp_repo
-    repo.create_remote("origin", url="git.test.com/test/repo.git")
+    repo.create_remote("origin", url=test_repo_url)
 
     args_dict = valid_args_dict
     args_dict["exclude-models"] = test_prof
@@ -113,10 +129,27 @@ def test_with_exclude_models(
         with pytest.raises(SystemExit, match="0"):
             cli_main()
 
-    # Verify the the correct files were included
+    # Verify that the profile was excluded
     commit = next(repo.iter_commits())
     assert test_cat_path in commit.stats.files
     assert len(commit.stats.files) == 1
 
     # Clean up the source repo
     clean(source, None)
+
+
+def test_with_no_sources(valid_args_dict: Dict[str, str], caplog: Any) -> None:
+    """Test with an invalid source argument."""
+    args_dict = valid_args_dict
+    args_dict["sources"] = ""
+
+    with patch("sys.argv", ["trestlebot", *args_dict_to_list(args_dict)]):
+        with pytest.raises(SystemExit, match="2"):
+            cli_main()
+
+    assert any(
+        record.levelno == logging.ERROR
+        and "Invalid args --sources: Must set at least one source to sync from."
+        in record.message
+        for record in caplog.records
+    )

trestlebot/entrypoints/sync_upstreams.py

@@ -88,8 +88,8 @@ def run(self, args: argparse.Namespace) -> None:
                     "--sources", "Must set at least one source to sync from."
                 )
 
-            # Assume that is skip_items is not set, then
-            # skip nothing and if include items is not set, then include all.
+            # Assume that if exclude_models is not set, then
+            # skip nothing and if include_models is not set, then include all.
             include_model_list: List[str] = ["*"]
             if args.include_models:
                 include_model_list = comma_sep_to_list(args.include_models)
@@ -98,7 +98,6 @@ def run(self, args: argparse.Namespace) -> None:
                 include_patterns=include_model_list,
             )
 
-            # Should be false is skip_validation is true
             validate: bool = not args.skip_validation
 
             sync_upstreams_task: TaskBase = SyncUpstreamsTask(