Adding additional testing

tests/conftest.py

@@ -11,11 +11,21 @@ def runner():
 def mock_env_vars(monkeypatch):
     monkeypatch.setenv("SN_USER_NAME", "user")
     monkeypatch.setenv("SN_PASSWORD", "password")
-    monkeypatch.setenv("SN_SET_USE_OAUTH", "False")
+    monkeypatch.setenv("SN_SET_USE_OAUTH", "false")
 
 
 @pytest.fixture
 def mock_empty_env_vars(monkeypatch):
     monkeypatch.setenv("SN_USER_NAME", "user")
     monkeypatch.setenv("SN_PASSWORD", "")
-    monkeypatch.setenv("SN_SET_USE_OAUTH", "False")
+    monkeypatch.setenv("SN_SET_USE_OAUTH", "false")
+
+
+@pytest.fixture
+def mock_oauth_env_vars(monkeypatch):
+    monkeypatch.setenv("SN_USER_NAME", "abc123")
+    monkeypatch.setenv("SN_PASSWORD", "super-secret")
+    monkeypatch.setenv("SN_SET_USE_OAUTH", "true")
+    monkeypatch.setenv("SN_SET_CLIENT_ID", "client_id")
+    monkeypatch.setenv("SN_SET_CLIENT_SECRET", "super-secure")
+    monkeypatch.setenv("SN_SET_GRANT_TYPE", "invalid")

tests/test_cli.py

@@ -280,3 +280,50 @@ def test_to_excel_valid(test_value, expected_value, runner):
 @pytest.mark.parametrize("test_value", [None, {}, [], 1])
 def test_to_excel_invalid(test_value):
     assert cli.to_excel(test_value, None) is False
+
+
+@mock.patch("sn_set.cli.get_set_diff")
+@mock.patch("sn_set.cli.get_install_order_new")
+@mock.patch("sn_set.cli.get_install_order")
+@mock.patch("sn_set.cli.get_update_sets")
+def test_cli_short_circuit(
+    mock_get_update_sets,
+    mock_get_install_order,
+    mock_new_install_order,
+    mock_set_diff,
+    runner,
+):
+    mock_set1 = [
+        {"name": "a set", "sys_id": "12345"},
+        {"name": "b set", "sys_id": "54321"},
+    ]
+    mock_set2 = [{"name": "a set", "sys_id": "54321"}]
+    mock_install_order = [{"name": "b set", "sys_id": "12345"}]
+
+    mock_get_update_sets.side_effect = [mock_set1, mock_set2]
+    mock_get_install_order.return_value = mock_install_order
+    mock_set_diff.side_effect = [["b set"], []]
+
+    test_source = "nyudev"
+    test_target = "nyuqa"
+    result = runner.invoke(
+        cli.main, ["--source", test_source, "--target", test_target, "--short"]
+    )
+
+    mock_get_update_sets.assert_any_call("nyudev")
+    mock_get_update_sets.assert_any_call("nyuqa")
+    mock_set_diff.assert_has_calls(
+        [
+            mock.call(["a set", "b set"], ["a set"], debug=False),
+            mock.call(["b set"], ["b set"]),
+        ]
+    )
+    mock_new_install_order.assert_not_called()
+
+    assert result.exit_code == 0
+    assert not result.exception
+    assert (
+        f"Begin retrieving update sets from "
+        f"source: {test_source} and target: {test_target}"
+    ) in result.output
+    assert "Short circuiting" in result.output

tests/test_requests_lib.py

@@ -285,3 +285,186 @@ def mock_make_request(instance_name, path_params):
         monkeypatch.setattr(requests_lib, "get_install_order_new", mock_make_request)
         r = requests_lib.get_install_order_new("nyudev", test_input)
         assert r == expected
+
+
+def test_client_factory_no_base_url(mock_oauth_env_vars):
+    from sn_set.requests_lib import client_factory
+
+    with pytest.raises(ValueError, match="base_url must be specified"):
+        client_factory()
+
+
+def test_client_factory_missing_password(mock_empty_env_vars):
+    from sn_set.requests_lib import client_factory
+
+    with pytest.raises(ValueError, match="Username or Password is empty"):
+        client_factory(base_url="https://test.com")
+
+
+def test_client_factory_missing_username(monkeypatch):
+    monkeypatch.setenv("SN_USER_NAME", "")
+    monkeypatch.setenv("SN_PASSWORD", "password")
+    monkeypatch.setenv("SN_SET_USE_OAUTH", "false")
+    from sn_set.requests_lib import client_factory
+
+    with pytest.raises(ValueError, match="Username or Password is empty"):
+        client_factory(base_url="https://test.com")
+
+
+@pytest.mark.parametrize(
+    "test_client_id,test_client_secret", {("", "client-secret"), ("client_id", "")}
+)
+def test_client_factory_oauth_settings_check(
+    test_client_id, test_client_secret, monkeypatch
+):
+    monkeypatch.setenv("SN_USER_NAME", "abc123")
+    monkeypatch.setenv("SN_PASSWORD", "PASSWORD")
+    monkeypatch.setenv("SN_SET_USE_OAUTH", "true")
+    monkeypatch.setenv("SN_SET_CLIENT_ID", test_client_id)
+    monkeypatch.setenv("SN_SET_CLIENT_SECRET", test_client_secret)
+
+    from sn_set.requests_lib import client_factory
+
+    with pytest.raises(
+        ValueError,
+        match="Client ID, Client Secret, and Grant Type are required to use OAuth2",
+    ):
+        client_factory(base_url="https://test.com")
+
+
+def test_client_factory_oauth_client(mock_oauth_env_vars, monkeypatch):
+    from authlib.integrations.requests_client import OAuth2Session
+
+    def mock_fetch_token(*args, **kwargs):
+        return None
+
+    monkeypatch.setattr(OAuth2Session, "fetch_token", mock_fetch_token)
+
+    from sn_set.requests_lib import client_factory
+
+    test_base_url: str = "https://test.com"
+
+    client, auth = client_factory(base_url=test_base_url)
+
+    assert client is not None
+    assert auth is None
+    assert isinstance(client, OAuth2Session)
+
+    from sn_set.requests_lib import context
+
+    assert context[test_base_url] is not None
+    assert context[test_base_url]["client"] == client
+
+    # clean up after test
+    del context[test_base_url]
+
+
+def test_client_factory_basic_auth_client(mock_env_vars):
+    import requests
+
+    from sn_set.requests_lib import client_factory
+
+    test_base_url: str = "https://test.com"
+
+    client, auth = client_factory(base_url=test_base_url)
+
+    assert client is not None
+    assert auth is not None
+    assert isinstance(auth, requests.auth.HTTPBasicAuth)
+
+    from sn_set.requests_lib import context
+
+    assert context[test_base_url] is not None
+    assert context[test_base_url]["client"] == client
+    assert context[test_base_url]["auth"] == auth
+
+    # clean up after test
+    del context[test_base_url]
+
+
+@mock.patch("sn_set.requests_lib.make_request")
+def test_get_install_order_new_400_fallback(mock_make_request):
+    """
+    Tests that get_install_order_new handles 400/414 errors by
+    splitting the request into individual calls.
+    """
+    # Payload with different dates to verify sorting in fallback
+    payload_a = [{"name": "set_a", "sys_updated_on": "2021-05-10 12:00:00"}]
+    payload_b = [{"name": "set_b", "sys_updated_on": "2021-05-05 12:00:00"}]
+
+    mocked_error = HTTPError(response=mock.Mock(status_code=400))
+
+    # Side effect: 1. Fail initial bulk call, 2. Return set_a, 3. Return set_b
+    mock_make_request.side_effect = [mocked_error, payload_a, payload_b]
+
+    from sn_set.requests_lib import get_install_order_new
+
+    # We expect the result to be sorted by sys_updated_on (set_b comes first)
+    result = get_install_order_new("nyudev", ["set_a", "set_b"])
+
+    assert len(result) == 2
+    assert result[0]["name"] == "set_b"
+    assert result[1]["name"] == "set_a"
+
+    # Verify we made exactly 3 calls
+    assert mock_make_request.call_count == 3
+
+    # Verify the fallback query structure for the first individual item
+    args, kwargs = mock_make_request.call_args_list[1]
+    assert "name=set_a" in kwargs["path_params"]["sysparm_query"]
+    assert "installed_fromISEMPTY" in kwargs["path_params"]["sysparm_query"]
+
+
+@pytest.mark.parametrize("status_code", [401, 404, 500])
+@mock.patch("sn_set.requests_lib.make_request")
+def test_get_install_order_new_other_errors(mock_make_request, status_code):
+    """
+    Tests that non-400/414 errors are not caught by the fallback
+    and are re-raised.
+    """
+    mock_error = HTTPError(response=mock.Mock(status_code=status_code))
+    mock_make_request.side_effect = mock_error
+
+    from sn_set.requests_lib import get_install_order_new
+
+    with pytest.raises(HTTPError):
+        get_install_order_new("nyudev", ["set_a"])
+
+
+def test_get_install_order_new_ordering_logic():
+    """
+    Directly tests that the order_sets call inside get_install_order_new
+    uses the correct field (sys_updated_on) compared to the old function.
+    """
+    mock_payload = [
+        {"name": "Latest", "sys_updated_on": "2021-12-31 23:59:59"},
+        {"name": "Earliest", "sys_updated_on": "2021-01-01 00:00:00"},
+    ]
+
+    from sn_set.requests_lib import order_sets
+
+    result = order_sets(mock_payload, order_by_field="sys_updated_on")
+
+    assert result[0]["name"] == "Earliest"
+    assert result[1]["name"] == "Latest"
+
+
+@mock.patch("sn_set.requests_lib.make_request")
+def test_get_install_order_new_fields_verification(mock_make_request):
+    """
+    Verifies that the function requests the specific fields
+    required for new update sets.
+    """
+    mock_make_request.return_value = []
+    from sn_set.requests_lib import get_install_order_new
+
+    get_install_order_new("nyudev", ["test_set"])
+
+    args, kwargs = mock_make_request.call_args
+    requested_fields = kwargs["path_params"]["sysparm_fields"].split(",")
+
+    # Should NOT have commit_date (as it's commented out in your source)
+    assert "commit_date" not in requested_fields
+    # Should have core fields
+    assert "sys_updated_on" in requested_fields
+    assert "name" in requested_fields

tests/test_settings.py

@@ -0,0 +1,23 @@
+from sn_set.settings import Settings
+
+
+class TestSettings:
+    def test_oauth_settings(self, mock_oauth_env_vars):
+        test_settings = Settings()
+
+        assert test_settings.get_user() == "abc123"
+        assert test_settings.get_password() == "super-secret"
+        assert test_settings.get_use_oauth()
+        assert test_settings.get_client_id() == "client_id"
+        assert test_settings.get_client_secret() == "super-secure"
+        assert test_settings.get_grant_type() == "password"
+
+    def test_basic_settings(self, mock_env_vars):
+        test_settings = Settings()
+
+        assert test_settings.get_user() == "user"
+        assert test_settings.get_password() == "password"
+        assert not test_settings.get_use_oauth()
+        assert test_settings.get_client_id() is None
+        assert test_settings.get_client_secret() is None
+        assert test_settings.get_grant_type() is None