Constants and Paths #110

ialarmedalien · 2024-07-03T23:46:26Z

Description of PR purpose/changes

set up a file with text constants
switched over to using the Path module for quite a few of the file/path-related functions

Jira Ticket / Issue

N/A

Testing Instructions

Details for how to test the PR:

Tests pass locally and in GitHub Actions

Dev Checklist:

My code follows the guidelines at https://sites.google.com/lbl.gov/trussresources/home?authuser=0
I have performed a self-review of my own code
I have commented my code, particularly in hard-to-understand areas
I have made corresponding changes to the documentation
My changes generate no new warnings
I have added tests that prove my fix is effective or that my feature works
New and existing unit tests pass locally with my changes
(Python) I have run Ruff on changed Python code
If appropriate, I have recompiled the app and added the updated StaticNarrativeImpl.py, StaticNarrativeServer.py, and compile_report.json to this PR.

…se pathlib instead.

ialarmedalien · 2024-07-03T23:49:32Z

lib/StaticNarrative/exceptions.py

@@ -1,3 +1,5 @@
+"""Custom StaticNarrative Errors."""


Adding some extra documentation to this file.

ialarmedalien · 2024-07-03T23:52:35Z

lib/StaticNarrative/narrative_ref.py

        err_msg = f"The Narrative {name} must be an integer > 0, not {number}"
+        if number is None or not str(number).isdigit():


Casting to int meant that if you put in values like 4.5, they would get converted to 4 and the ref would appear valid, so I made this a bit more strict.

ialarmedalien · 2024-07-03T23:53:38Z

lib/StaticNarrative/narrative_ref.py

+        missing_parts = [part for part in part_name if part not in ref]
+        if missing_parts:
+            msg = "Missing keys required to create a NarrativeRef: " + ", ".join(missing_parts)


ensure that it isn't possible to do NarrativeRef({"wsid": 12345}) and get a valid NarrativeRef out

ialarmedalien · 2024-07-03T23:55:14Z

test/mocks.py

@@ -100,11 +100,11 @@ def mock_adapter(request: requests.Request) -> requests.Response:
                tag = params[0]["tag"]
                ids = params[0]["ids"]
                result = [_get_fake_nms_info(tag, ids)]
-            response._content = bytes(json.dumps({"result": result, "version": "1.1"}), "UTF-8")
+            response._content = bytes(json.dumps({"result": result, "version": "1.1"}), "UTF-8")  # noqa: SLF001


prevent warnings when accessing response._content

ialarmedalien · 2024-07-03T23:55:39Z

test/mocks.py

-def mock_auth_ok(user_id, token):
-    pass
-
-
-def mock_auth_bad_token(token):
-    pass
-
-
-def mock_ws_narrative_fetch(narrative_ref):
-    pass
-
-
-def mock_ws_narrative_fetch_forbidden(narrative_ref):
-    pass
-
-
-def mock_ws_info(ws_id):
-    pass
-
-
-def mock_ws_info_unauth(ws_id):
-    pass


none of these are used

ialarmedalien · 2024-07-03T23:56:03Z

test/run_tests.sh

-# ensure there's a valid scratch dir and an assets dir
-mkdir -p "$current_dir"/../scratch/nginx
-


no longer need this (forgot to delete it in a previous PR)

ialarmedalien · 2024-07-03T23:57:42Z

test/test_config.py

+    monkeypatch.setattr(Path, "is_absolute", starts_with_slash)
+    monkeypatch.setattr(Path, "is_dir", starts_with_slash)
+    monkeypatch.setattr(Path, "resolve", absolutify)


replace the old mocks with these monkey patched versions (arguably a bit simpler?)

ialarmedalien · 2024-07-03T23:59:58Z

test/test_static_narrative_server.py

@@ -7,8 +7,6 @@

 from test.mocks import set_up_ok_mocks

-USER_ID = "some_user"


switching out USER_ID for fake_user from the conftest

ialarmedalien · 2024-07-04T00:00:08Z

test/test_narrative_util.py

-USER_ID = "some_user"
-TOKEN = "some_token"  # noqa: S105


codecov · 2024-07-05T18:36:40Z

Codecov Report

Attention: Patch coverage is 94.51220% with 9 lines in your changes missing coverage. Please review.

Project coverage is 72.47%. Comparing base (4cd0de1) to head (925e208).
Report is 1 commits behind head on develop.

Files	Patch %	Lines
lib/StaticNarrative/exporter/icon_util.py	91.22%	5 Missing ⚠️
lib/StaticNarrative/creator.py	94.11%	2 Missing ⚠️
lib/StaticNarrative/narrative_ref.py	80.00%	2 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff             @@
##           develop     #110      +/-   ##
===========================================
+ Coverage    70.67%   72.47%   +1.79%     
===========================================
  Files           22       24       +2     
  Lines         1497     1544      +47     
===========================================
+ Hits          1058     1119      +61     
+ Misses         439      425      -14

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

briehl · 2024-07-08T18:33:22Z

test/test_config.py

-        yield mock_isdir
+def absolutify(self: Path) -> Path:
+    """Generate an absolute version of a path."""
+    return Path("/absolute") / self


That's pretty absolute!

Move remaining os.paths over to Path

Icon tests

test/test_icon_util.py

+from unittest.mock import MagicMock
+
+import pytest
+import StaticNarrative.exporter.icon_util


consolidate constants

f1d8bc9

ialarmedalien requested a review from briehl July 3, 2024 23:46

ialarmedalien self-assigned this Jul 3, 2024

fixing some missed constants. Updating some of the os.path stuff to u…

4f4ac77

…se pathlib instead.

ialarmedalien force-pushed the constants branch from 9809c9a to 4f4ac77 Compare July 3, 2024 23:47

ialarmedalien commented Jul 3, 2024

View reviewed changes

ialarmedalien commented Jul 4, 2024

View reviewed changes

ialarmedalien added 2 commits July 5, 2024 11:34

Fix temp dir for testing

30f3b21

Adding some tests for icon retrieval

d845221

ialarmedalien added 4 commits July 8, 2024 05:42

Move remaining os.path over to Path

522d7d6

Adding pytest-mock to requirements

48bd5c7

Prevent config edit during test from breaking a load of other tests

f2c34aa

Fix Path() in test

1d433ad

briehl approved these changes Jul 8, 2024

View reviewed changes

ialarmedalien added 2 commits July 8, 2024 13:13

Merge pull request #112 from kbaseapps/remaining_paths

454dc98

Move remaining os.paths over to Path

Merge pull request #111 from kbaseapps/icon_tests

925e208

Icon tests

github-advanced-security bot found potential problems Jul 8, 2024

View reviewed changes

ialarmedalien merged commit 2ce71f7 into develop Jul 8, 2024
12 checks passed

ialarmedalien deleted the constants branch July 8, 2024 20:17

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Constants and Paths #110

Constants and Paths #110

ialarmedalien commented Jul 3, 2024

ialarmedalien Jul 3, 2024

ialarmedalien Jul 3, 2024

ialarmedalien Jul 3, 2024

ialarmedalien Jul 3, 2024

ialarmedalien Jul 3, 2024

ialarmedalien Jul 3, 2024

ialarmedalien Jul 3, 2024

ialarmedalien Jul 3, 2024

ialarmedalien Jul 4, 2024

codecov bot commented Jul 5, 2024 •

edited

Loading

briehl Jul 8, 2024

		err_msg = f"The Narrative {name} must be an integer > 0, not {number}"
		if number is None or not str(number).isdigit():

		# ensure there's a valid scratch dir and an assets dir
		mkdir -p "$current_dir"/../scratch/nginx

		@@ -7,8 +7,6 @@

		from test.mocks import set_up_ok_mocks

		USER_ID = "some_user"

Constants and Paths #110

Constants and Paths #110

Conversation

ialarmedalien commented Jul 3, 2024

Description of PR purpose/changes

Jira Ticket / Issue

Testing Instructions

Dev Checklist:

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov bot commented Jul 5, 2024 • edited Loading

Codecov Report

Choose a reason for hiding this comment

codecov bot commented Jul 5, 2024 •

edited

Loading