Improved templating audits #238

kenodegard · 2024-12-17T06:16:13Z

Description

Depends on #243

Whoa this became kinda complicated...no wonder I originally skipped this stuff.

I was reminded (in conda/conda-libmamba-solver#588 (comment)) that I had chosen not to over engineered the templating engine but with the templates becoming more complex (and others besides myself monitoring the templating jobs) we might as well include all the bells and whistles to help ourselves when debugging.

Old Audit
New Audit

This change introduces a few key improvements:

we now watch for context usage so we can warn the user about missing variables, the context has 4 possible states:

State	Description
	a variable that has been successfully used in the template
	a variable that has not been defined but has a fallback value in the template
	a variable that has not been defined and has no fallback value
	a variable that is not used in the template

if the templating produces errors we will leave the audit open by default, this will make the errors more noticeable
the template stubs table also includes counts of how many times the stubs are referenced

…mLoader

for more information, see https://pre-commit.ci

kenodegard · 2024-12-17T15:09:14Z

combine-durations/action.py

pre-commit/ruff changes

kenodegard · 2024-12-17T15:14:14Z

template-files/action.py

+    @cache
+    def _get_emoji_style(self) -> tuple[str, str]:
+        match self:
+            case self.UNUSED:
+                return ":warning-emoji:", "yellow"
+            case self.MISSING:
+                return ":cross_mark:", "red"
+            case self.USED:
+                return ":white_check_mark:", "green"
+            case self.CONTEXT:
+                return ":books:", "blue"
+            case self.OPTIONAL:
+                return ":heavy_plus_sign:", "yellow"
+            case _:
+                raise ValueError("Invalid TemplateState")


or is the old-school way more clear? it's more concise 🤷🏼‍♂️

Suggested change

@cache

def _get_emoji_style(self) -> tuple[str, str]:

match self:

case self.UNUSED:

return ":warning-emoji:", "yellow"

case self.MISSING:

return ":cross_mark:", "red"

case self.USED:

return ":white_check_mark:", "green"

case self.CONTEXT:

return ":books:", "blue"

case self.OPTIONAL:

return ":heavy_plus_sign:", "yellow"

case _:

raise ValueError("Invalid TemplateState")

@cache

def _get_emoji_style(self) -> tuple[str, str]:

try:

return {

self.UNUSED: (":warning-emoji:", "yellow"),

self.MISSING: (":cross_mark:", "red"),

self.USED: (":white_check_mark:", "green"),

self.CONTEXT: (":books:", "blue"),

self.OPTIONAL: (":heavy_plus_sign:", "yellow"),

}[self]

except KeyError:

raise ValueError("Invalid TemplateState")

No, I like the match ... case ... statements.

kenodegard · 2024-12-17T15:16:49Z

template-files/action.py

+                # GitHub repository name or local directory
+                r"\w+/\w+|\..+": {


this allows us to fetch templates from the local file system, not just those checked into GitHub repos

kenodegard · 2024-12-17T15:18:12Z

template-files/action.py

+class LocalContents:
+    # mirror GitHub contents object
+    def __init__(self, path: Path):
+        self.decoded_content = path.read_text().encode()
+
+
+class LocalRepository:
+    # mirror GitHub repository object
+    def __init__(self, *paths: str | os.PathLike[str] | Path) -> None:
+        self.path = Path(*paths).resolve()
+        if not paths or not self.path.is_dir():
+            raise FileNotFoundError(f"{self.path} is not a directory")
+
+        # constants
+        self.html_url = f"file://{self.path}"
+        self.user = "<local>"
+        self.name = "<local>"
+
+    def get_contents(self, path: str) -> LocalContents:
+        return LocalContents(self.path / path)


these mirror the GitHub Repository and Contents objects allowing us to use templates from the local file system (this is useful for testing purposes)

while useful for testing idk if we should keep it, feels like a hack that will only hurt in the future

Well, it doesn't look like this is being tested anyways (at least in CI), so if it helps with testing it locally I don't see a reason for removing it.

travishathaway · 2025-01-03T12:47:19Z

@kenodegard,

Maybe a little outside the scope of this pull request, but why isn't the code in this repository being tested? 😅

If you could, maybe now would be a good time to introduce it if only for this code that is changing in this pull request.

travishathaway · 2025-01-03T12:53:24Z

I created a separate issue for adding a testing actions:

Add an action to run tests as part of pull requests (and some trivial "starter" tests) #242

kenodegard · 2025-01-03T16:11:21Z

Maybe a little outside the scope of this pull request, but why isn't the code in this repository being tested? 😅

GitHub Actions/Workflows are not exactly easy to test in many cases since they're always running in "production", this GHA is probably an easier one to test since the action defers to a standalone script, but yes it would be good to figure something out testing wise

github-actions · 2025-01-10T05:29:06Z

Template Success

Templating Audit

🔄 Fetching files from ./templates
- ✅ file1.txt → .github_cache/template-files/success/file1.txt
  - ✅ (used) stub1.txt
  - ✅ (used) stub2.txt
  - ❌ (missing) stub3.txt
- ⚠️ file2.txt → .github_cache/template-files/success/file2.txt
  - ✅ (used) stub2.txt
  - ❌ (missing) stub3.txt
  - 📚 (context) variable=value
  - ➕ (optional) optional=

Stub	State	Count
`stub1.txt`	✅ (used)	1
`stub2.txt`	✅ (used)	2
`stub3.txt`	❌ (missing)	2

Template Error

Templating Audit

🔄 Fetching files from ./templates
- ✅ file1.txt → .github_cache/template-files/error/file1.txt
  - ✅ (used) stub1.txt
  - ✅ (used) stub2.txt
  - ❌ (missing) stub3.txt
- ❌ Context missing file2.txt → .github_cache/template-files/error/file2.txt
  - ✅ (used) stub2.txt
  - ❌ (missing) stub3.txt
  - ❌ (missing) variable=
  - ➕ (optional) optional=

Stub	State	Count
`stub1.txt`	✅ (used)	1
`stub2.txt`	✅ (used)	2
`stub3.txt`	❌ (missing)	2

Got 1 error(s)

…it-context

kenodegard added 7 commits December 16, 2024 21:34

Include workflow name in cache key

9daa717

Update pyproject.toml

12999d6

Refactor StubLoader into SpyEnvironment, SpyContext, and SpyFileSyste…

abd1807

…mLoader

Distinguish between required and optional context

62dd42b

Default details to open when errors occur

dcf2536

Fix get_source auditing

2d0b698

Rename Spy to Audit

d2cebef

kenodegard requested a review from a team as a code owner December 17, 2024 06:16

conda-bot added the cla-signed [bot] added once the contributor has signed the CLA label Dec 17, 2024

[pre-commit.ci] auto fixes from pre-commit.com hooks

d806acb

for more information, see https://pre-commit.ci

dbast previously approved these changes Dec 17, 2024

View reviewed changes

kenodegard commented Dec 17, 2024

View reviewed changes

combine-durations/action.py Outdated

Copy link

Contributor Author

kenodegard Dec 17, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

pre-commit/ruff changes

kenodegard commented Dec 17, 2024

View reviewed changes

Rename spy contextmanager to audit

81cd709

kenodegard dismissed dbast’s stale review via 81cd709 December 17, 2024 15:15

kenodegard commented Dec 17, 2024

View reviewed changes

kenodegard added 5 commits January 6, 2025 23:13

Initialize test suite

2f0b54b

Add test_dump_summary

7b5bda7

Parametrize TemplateState tests

3476500

Add initial test for template_file

e9046a8

Initialize test suite

f9ab207

kenodegard mentioned this pull request Jan 8, 2025

Add tests & CI framework #243

Merged

kenodegard added 4 commits January 7, 2025 21:08

Install all dependencies

132e3cf

Upload coverage

0d9ef43

Add badges

06d63b9

Update README.md

ee03d8e

kenodegard mentioned this pull request Jan 9, 2025

Add analyze step #245

Merged

kenodegard added 6 commits January 9, 2025 15:41

Update existing failures

92e1cce

Merge branch 'scheduled-tests' into template-filess-audit-context

09b0ba2

Catch IsADirectoryError

f4ee1ae

Typing

115502b

Pytest ids

ca383dd

Update README

3db7491

kenodegard force-pushed the template-filess-audit-context branch 2 times, most recently from caf37f1 to eb76256 Compare January 10, 2025 03:03

Add GHA test

8eaa05a

kenodegard force-pushed the template-filess-audit-context branch from eb76256 to 8eaa05a Compare January 10, 2025 03:10

Fix stub audit

cf43b10

kenodegard force-pushed the template-filess-audit-context branch 8 times, most recently from bc0e4a9 to 55f3449 Compare January 10, 2025 05:07

Comment template-files summary

c595976

kenodegard force-pushed the template-filess-audit-context branch from 55f3449 to c595976 Compare January 10, 2025 05:12

This comment was marked as outdated.

Sign in to view

Update tests

b2f7942

conda deleted a comment from github-actions bot Jan 10, 2025

Use a sticky comment instead

73ae56d

kenodegard force-pushed the template-filess-audit-context branch from a72247b to 73ae56d Compare January 10, 2025 05:29

Merge remote-tracking branch 'upstream/main' into template-filess-aud…

619f950

…it-context

kenodegard marked this pull request as ready for review January 10, 2025 15:56

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Improved templating audits #238

Improved templating audits #238

kenodegard commented Dec 17, 2024 •

edited

Loading

kenodegard Dec 17, 2024

kenodegard Dec 17, 2024

travishathaway Jan 3, 2025

kenodegard Dec 17, 2024

kenodegard Dec 17, 2024

kenodegard Dec 17, 2024

travishathaway Jan 3, 2025

travishathaway commented Jan 3, 2025

travishathaway commented Jan 3, 2025

kenodegard commented Jan 3, 2025

This comment was marked as outdated.

github-actions bot commented Jan 10, 2025 •

edited

Loading

		# GitHub repository name or local directory
		r"\w+/\w+\|\..+": {

Improved templating audits #238

Are you sure you want to change the base?

Improved templating audits #238

Conversation

kenodegard commented Dec 17, 2024 • edited Loading

Description

kenodegard Dec 17, 2024

Choose a reason for hiding this comment

kenodegard Dec 17, 2024

Choose a reason for hiding this comment

travishathaway Jan 3, 2025

Choose a reason for hiding this comment

kenodegard Dec 17, 2024

Choose a reason for hiding this comment

kenodegard Dec 17, 2024

Choose a reason for hiding this comment

kenodegard Dec 17, 2024

Choose a reason for hiding this comment

travishathaway Jan 3, 2025

Choose a reason for hiding this comment

travishathaway commented Jan 3, 2025

travishathaway commented Jan 3, 2025

kenodegard commented Jan 3, 2025

This comment was marked as outdated.

github-actions bot commented Jan 10, 2025 • edited Loading

Template Success

Template Error

kenodegard commented Dec 17, 2024 •

edited

Loading

github-actions bot commented Jan 10, 2025 •

edited

Loading