Add missing TestWorkflowSecurity and TestEdgeCases classes to iteration status emails workflow tests by Copilot · Pull Request #177 · JaclynCodes/Symphonic-Joules

Copilot · 2026-01-25T02:05:48Z

💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.

Summary by cubic

Renames TestSecurityBestPractices to TestWorkflowSecurity and adds a TestEdgeCases suite to cover YAML and workflow edge cases, per review on #171. No behavior changes; this aligns the refactor with feedback to unblock #171.

^{Written for commit 625eb2c. Summary will update on new commits.}

…EdgeCases class Co-authored-by: JaclynCodes <218383634+JaclynCodes@users.noreply.github.com>

…_missing_dashboard Co-authored-by: JaclynCodes <218383634+JaclynCodes@users.noreply.github.com>

coderabbitai · 2026-01-25T02:14:53Z

Important

Review skipped

Bot user detected.

To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Note

Other AI code review bot(s) detected

CodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review.

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

cubic-dev-ai

No issues found across 1 file

Copilot

Pull request overview

Updates the iteration status emails workflow test module to match the common workflow test structure expected by cross-file consistency tests.

Changes:

Renamed TestSecurityBestPractices to TestWorkflowSecurity for naming consistency across workflow test modules.
Added a TestEdgeCases class with additional YAML/workflow edge-case checks.

tests/workflows/test_iteration_status_emails_workflow.py

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Signed-off-by: Misfit <218383634+JaclynCodes@users.noreply.github.com>

JaclynCodes · 2026-01-25T02:19:42Z

@copilot apply changes based on the comments in this thread🚀

cubic-dev-ai

1 issue found across 1 file (changes from recent commits).

Prompt for AI agents (all issues)


Check if these issues are valid — if so, understand the root cause of each and fix them.


<file name="tests/workflows/test_iteration_status_emails_workflow.py">

<violation number="1" location="tests/workflows/test_iteration_status_emails_workflow.py:393">
P2: The required key check treats only the literal "on" key as valid, but YAML may parse it as True (noted elsewhere in this file). This will cause false failures on valid workflows that use the boolean key.</violation>
</file>

_{Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.}

tests/workflows/test_iteration_status_emails_workflow.py

…error handling Co-authored-by: JaclynCodes <218383634+JaclynCodes@users.noreply.github.com>

…k pattern Co-authored-by: JaclynCodes <218383634+JaclynCodes@users.noreply.github.com>

…pecific assertions Co-authored-by: JaclynCodes <218383634+JaclynCodes@users.noreply.github.com>

Co-authored-by: JaclynCodes <218383634+JaclynCodes@users.noreply.github.com>

…sts and remove trailing whitespace Co-authored-by: JaclynCodes <218383634+JaclynCodes@users.noreply.github.com>

Copilot · 2026-01-25T02:37:25Z

@copilot apply changes based on the comments in this thread

Applied all changes from review #3703102668:

Added jobs fixture matching other workflow tests
Simplified test_no_duplicate_job_names to use jobs fixture (consistent with test_static_workflow.py)
Removed trailing whitespace from blank lines

All 39 tests pass. Changes in commit 9a9acd3.

cubic-dev-ai

1 issue found across 1 file (changes from recent commits).

Prompt for AI agents (all issues)


Check if these issues are valid — if so, understand the root cause of each and fix them.


<file name="tests/workflows/test_iteration_status_emails_workflow.py">

<violation number="1" location="tests/workflows/test_iteration_status_emails_workflow.py:359">
P2: This test no longer detects duplicate job names because it checks uniqueness on a dict that has already discarded duplicates. Use a loader that raises on duplicate keys so the test still fails when the YAML contains duplicate job entries.</violation>
</file>

_{Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.}

cubic-dev-ai · 2026-01-25T02:38:46Z

tests/workflows/test_iteration_status_emails_workflow.py

+                    assert leading_spaces % 2 == 0, \
+                        f"Line {i} has indentation that is not a multiple of 2 spaces"
+
+    def test_no_duplicate_job_names(self, jobs):


P2: This test no longer detects duplicate job names because it checks uniqueness on a dict that has already discarded duplicates. Use a loader that raises on duplicate keys so the test still fails when the YAML contains duplicate job entries.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At tests/workflows/test_iteration_status_emails_workflow.py, line 359: <comment>This test no longer detects duplicate job names because it checks uniqueness on a dict that has already discarded duplicates. Use a loader that raises on duplicate keys so the test still fails when the YAML contains duplicate job entries.</comment> <file context> @@ -347,26 +355,11 @@ def test_consistent_indentation(self, workflow_raw): - # Parsing with UniqueKeyLoader will raise if any duplicate keys are present in the YAML. - yaml.load(workflow_raw, Loader=UniqueKeyLoader) + + def test_no_duplicate_job_names(self, jobs): + """Test that there are no duplicate job names.""" + job_names = list(jobs.keys()) </file context>

tests/workflows/test_iteration_status_emails_workflow.py

JaclynCodes · 2026-01-25T22:50:36Z

@copilot apply changes based on the comments in this thread

…ys() as requested Co-authored-by: JaclynCodes <218383634+JaclynCodes@users.noreply.github.com>

Copilot · 2026-01-25T22:51:24Z

@copilot apply changes based on the comments in this thread🚀

All changes from review thread #3703096165 have been applied. The implementation now uses workflow_content['jobs'].keys() as clarified in comment 2724788844. Commit 625eb2c.

@copilot apply changes based on the comments in this thread

Applied the requested change to use workflow_content['jobs'].keys() instead of the jobs fixture. All 39 tests pass. Commit 625eb2c.

@copilot Check if these issues are valid — if so, understand the root cause of each and fix them.

...

The issue was clarified in comment 2724788844. Changed the implementation to use workflow_content['jobs'].keys() as requested. All tests pass. Commit 625eb2c.

JaclynCodes

👀🚀

Resolved

Copilot

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 4 comments.

Copilot · 2026-02-08T16:18:40Z

tests/workflows/test_iteration_status_emails_workflow.py

+    def test_no_duplicate_step_ids(self, workflow_content):
+        """Test that step IDs are unique within each job."""
+        jobs = workflow_content.get('jobs', {})
+        for job_name, job_config in jobs.items():
+            steps = job_config.get('steps', [])
+            step_ids = [s.get('id') for s in steps if 'id' in s]
+            assert len(step_ids) == len(set(step_ids)), \
+                f"Duplicate step IDs in job '{job_name}'"


test_no_duplicate_step_ids assumes every step is a dict; if the YAML contains an empty step (None) or a non-mapping value, if 'id' in s will raise. Consider asserting isinstance(step, dict) (or filtering to dicts) before inspecting keys so the test fails cleanly on malformed steps.

Copilot · 2026-02-08T16:18:41Z

tests/workflows/test_iteration_status_emails_workflow.py

+@pytest.fixture(scope='module')
+def jobs(workflow_content):
+    """
+    Module-scoped fixture for jobs configuration.
+    """
+    return workflow_content.get('jobs', {})


The new jobs fixture is not referenced anywhere in this module. Either remove it to avoid dead test code, or switch the edge-case tests to accept jobs and use it (matching the pattern in other workflow test modules).

Copilot · 2026-02-08T16:18:41Z

tests/workflows/test_iteration_status_emails_workflow.py

+
+    def test_no_duplicate_job_names(self, workflow_content):
+        """Test that there are no duplicate job names."""
+        job_names = list(workflow_content['jobs'].keys())


test_no_duplicate_job_names indexes workflow_content['jobs'], which will raise a KeyError (test error) if the YAML is missing the jobs key. Prefer asserting jobs exists first (or using the jobs fixture / workflow_content.get('jobs')) so the test fails with a clear assertion message instead of erroring.

Suggested change

job_names = list(workflow_content['jobs'].keys())

jobs = workflow_content.get('jobs')

assert jobs is not None, "Workflow must define jobs"

job_names = list(jobs.keys())

Copilot · 2026-02-08T16:18:41Z

tests/workflows/test_iteration_status_emails_workflow.py

+        missing = required_keys.difference(workflow_content.keys())
+        assert not missing, f"Workflow is missing required top-level keys: {', '.join(sorted(missing))}"
+        # Verify trigger configuration exists (can be 'on' or True)
+        has_triggers = 'on' in workflow_content or True in workflow_content.keys()


has_triggers = 'on' in workflow_content or True in workflow_content.keys() can produce false positives because True == 1 in Python (so a numeric key 1 would satisfy it). Use the same approach as the other trigger tests (workflow_content.get('on') or workflow_content.get(True)) or check boolean keys with identity (e.g., any(k is True for k in keys)).

Suggested change

has_triggers = 'on' in workflow_content or True in workflow_content.keys()

has_triggers = (

'on' in workflow_content

or any(k is True for k in workflow_content.keys())

)

Initial plan

ccb286c

Copilot AI assigned Copilot and JaclynCodes Jan 25, 2026

Copilot AI mentioned this pull request Jan 25, 2026

Big Refactor 🧱🗡️🎵➰ #171

Merged

Copilot started work on behalf of JaclynCodes January 25, 2026 02:06 View session

Copilot AI and others added 2 commits January 25, 2026 02:08

Rename TestSecurityBestPractices to TestWorkflowSecurity and add Test…

7297a02

…EdgeCases class Co-authored-by: JaclynCodes <218383634+JaclynCodes@users.noreply.github.com>

Use workflow_content fixture for consistency in test_workflow_handles…

e385b83

…_missing_dashboard Co-authored-by: JaclynCodes <218383634+JaclynCodes@users.noreply.github.com>

Copilot AI changed the title ~~[WIP] WIP Address feedback from review on big refactor PR~~ Add missing TestWorkflowSecurity and TestEdgeCases classes to iteration status emails workflow tests Jan 25, 2026

Copilot AI requested a review from JaclynCodes January 25, 2026 02:11

Copilot finished work on behalf of JaclynCodes January 25, 2026 02:11

JaclynCodes approved these changes Jan 25, 2026

View reviewed changes

JaclynCodes marked this pull request as ready for review January 25, 2026 02:14

Copilot AI review requested due to automatic review settings January 25, 2026 02:14

Copilot started reviewing on behalf of JaclynCodes January 25, 2026 02:15 View session

cubic-dev-ai bot reviewed Jan 25, 2026

View reviewed changes

Copilot AI reviewed Jan 25, 2026

View reviewed changes

tests/workflows/test_iteration_status_emails_workflow.py Show resolved Hide resolved

tests/workflows/test_iteration_status_emails_workflow.py Outdated Show resolved Hide resolved

tests/workflows/test_iteration_status_emails_workflow.py Outdated Show resolved Hide resolved

JaclynCodes and others added 2 commits January 24, 2026 21:18

Update test_iteration_status_emails_workflow.py

de7e8e3

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Signed-off-by: Misfit <218383634+JaclynCodes@users.noreply.github.com>

Update test_iteration_status_emails_workflow.py

d0e4a25

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Signed-off-by: Misfit <218383634+JaclynCodes@users.noreply.github.com>

Copilot started work on behalf of JaclynCodes January 25, 2026 02:19 View session

cubic-dev-ai bot reviewed Jan 25, 2026

View reviewed changes

tests/workflows/test_iteration_status_emails_workflow.py Outdated Show resolved Hide resolved

Copilot AI and others added 3 commits January 25, 2026 02:21

Improve test_workflow_handles_missing_dashboard to verify parse step …

81c07ed

…error handling Co-authored-by: JaclynCodes <218383634+JaclynCodes@users.noreply.github.com>

Fix code review issues: correct True key check and simplify file chec…

16640ba

…k pattern Co-authored-by: JaclynCodes <218383634+JaclynCodes@users.noreply.github.com>

Improve test robustness: use step id for reliable matching and more s…

b0d5e54

…pecific assertions Co-authored-by: JaclynCodes <218383634+JaclynCodes@users.noreply.github.com>

Copilot AI requested a review from JaclynCodes January 25, 2026 02:25

Copilot finished work on behalf of JaclynCodes January 25, 2026 02:25

Copilot started work on behalf of JaclynCodes January 25, 2026 02:25 View session

Fix formatting: add blank line between test methods

cfc5882

Co-authored-by: JaclynCodes <218383634+JaclynCodes@users.noreply.github.com>

This comment was marked as duplicate.

Sign in to view

Fix test_no_duplicate_job_names to match pattern in other workflow te…

9a9acd3

…sts and remove trailing whitespace Co-authored-by: JaclynCodes <218383634+JaclynCodes@users.noreply.github.com>

Copilot finished work on behalf of JaclynCodes January 25, 2026 02:37

cubic-dev-ai bot reviewed Jan 25, 2026

View reviewed changes

JaclynCodes requested changes Jan 25, 2026

View reviewed changes

tests/workflows/test_iteration_status_emails_workflow.py Show resolved Hide resolved

tests/workflows/test_iteration_status_emails_workflow.py Show resolved Hide resolved

Copilot started work on behalf of JaclynCodes January 25, 2026 22:45 View session

Copilot AI requested a review from JaclynCodes January 25, 2026 22:47

Copilot finished work on behalf of JaclynCodes January 25, 2026 22:47

Copilot started work on behalf of JaclynCodes January 25, 2026 22:50 View session

Change test_no_duplicate_job_names to use workflow_content['jobs'].ke…

625eb2c

…ys() as requested Co-authored-by: JaclynCodes <218383634+JaclynCodes@users.noreply.github.com>

JaclynCodes approved these changes Jan 25, 2026

View reviewed changes

Copilot AI requested a review from JaclynCodes January 25, 2026 22:52

Copilot finished work on behalf of JaclynCodes January 25, 2026 22:52

Copilot started work on behalf of JaclynCodes January 25, 2026 22:52 View session

JaclynCodes approved these changes Jan 25, 2026

View reviewed changes

This comment has been minimized.

Sign in to view

Copilot AI requested a review from JaclynCodes January 25, 2026 22:54

Copilot finished work on behalf of JaclynCodes January 25, 2026 22:54

JaclynCodes previously requested changes Jan 25, 2026

View reviewed changes

Copilot started work on behalf of JaclynCodes January 25, 2026 22:55 View session

Copilot AI requested a review from JaclynCodes January 25, 2026 22:56

Copilot finished work on behalf of JaclynCodes January 25, 2026 22:56

JaclynCodes approved these changes Jan 25, 2026

View reviewed changes

JaclynCodes approved these changes Jan 30, 2026

View reviewed changes

JaclynCodes requested a review from Copilot February 8, 2026 16:05

Copilot started reviewing on behalf of JaclynCodes February 8, 2026 16:05 View session

Copilot AI reviewed Feb 8, 2026

View reviewed changes

-        job_names = list(workflow_content['jobs'].keys())
+        jobs = workflow_content.get('jobs')
+        assert jobs is not None, "Workflow must define jobs"
+        job_names = list(jobs.keys())

Conversation

Copilot AI commented Jan 25, 2026 • edited by cubic-dev-ai bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary by cubic

Uh oh!

coderabbitai bot commented Jan 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Review skipped

Other AI code review bot(s) detected

Uh oh!

cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

JaclynCodes commented Jan 25, 2026

Uh oh!

cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

This comment was marked as duplicate.

Uh oh!

Copilot AI commented Jan 25, 2026

Uh oh!

cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

Uh oh!

cubic-dev-ai bot Jan 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

JaclynCodes commented Jan 25, 2026

Uh oh!

Copilot AI commented Jan 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

This comment has been minimized.

JaclynCodes left a comment

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Feb 8, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 8, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 8, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 8, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Copilot AI commented Jan 25, 2026 •

edited by cubic-dev-ai bot

Loading

coderabbitai bot commented Jan 25, 2026 •

edited

Loading

cubic-dev-ai bot Jan 25, 2026 •

edited

Loading

Copilot AI commented Jan 25, 2026 •

edited

Loading