Refactor/implement rewrite coverage #858

wheresdiasd · 2025-11-08T12:11:43Z

The pull requests consist in adding coverage for the sprint-3 coursework "IMPLEMENT AND REWRITE".

When users open a PR with the pattern 'coursework/sprint-3-implement-and-rewrite from their forks, it should run the expected tests for the coursework scenarios in a way the Reviewer have a smooth perspective of the written test results.

In addition, introduces a shell script that compares the implementation with the test cases to see if their match in equality of test cases.

The summary looks like below

And that is how the checks tab look like where we can see if the tests are actually passing or not.

github-actions · 2025-11-08T12:12:04Z

Your PR's title isn't in the expected format.

Please check the expected title format, and update yours to match.

Reason: Wrong number of parts separated by |s

If this PR is not coursework, please add the NotCoursework label (and message on Slack in #cyf-curriculum or it will probably not be noticed).

- Add Jest coverage configuration with 100% threshold - Create inline test verification script that: - Runs inline assertion tests with Node.js - Detects console.assert failures - Verifies test parity between inline and Jest tests - Reports disparities in both directions - Add GitHub Actions workflow that: - Triggers on coursework/sprint-3-implement-and-rewrite* branches - Runs inline tests with parity check - Runs Jest tests with coverage - Generates coverage report on PRs

- Set branches, lines, statements to 40% - Keep functions at 100% to ensure every function is tested - More practical for educational purposes while ensuring core functionality is covered

- Remove ArtiomTr/jest-coverage-report-action (was failing) - Add simple coverage summary to GitHub Actions summary - Upload coverage artifacts with if: always() to ensure they're saved even on failure

- Add inline test results table to summary (passed/failed counts) - Add test parity comparison table showing mismatches - Generate HTML coverage reports for download - Parse and display coverage metrics in summary with threshold checks - Visual status indicators (✅/❌) for all metrics - Clear instructions to download HTML coverage report artifact

- Replace custom parsing with maintained jest-coverage-comment action - Displays coverage badge, summary table, and file-by-file coverage - Shows in both PR comments and GitHub Actions summary - More reliable than manual JSON parsing

- Add peaceiris/actions-gh-pages to deploy HTML reports - Coverage reports accessible at github.io URL per branch - Add coverage report link to GitHub Actions summary - No need to download artifacts, view directly in browser

- Remove GitHub Pages deployment - Remove MishaKav/jest-coverage-comment - Let ArtiomTr action run tests and generate coverage itself - Simpler workflow with just inline tests + ArtiomTr coverage report

- Run Jest with required flags: --coverage --ci --json --testLocationInResults --outputFile=report.json - Pass report.json to ArtiomTr action via coverage-file parameter - Matches the documented example from ArtiomTr/jest-coverage-report-action

- Add pull-requests: write permission to allow action to comment on PRs - ArtiomTr action will now automatically post coverage reports as PR comments - Comments will include coverage summary and file-by-file breakdown

- Remove pull-requests: write permission - Coverage report will only show in GitHub Actions Summary - Avoids permission issues when forking to upstream repos

github-actions · 2025-11-20T09:02:59Z

Your PR's title isn't in the expected format.

Please check the expected title format, and update yours to match.

Reason: Wrong number of parts separated by |s

If this PR is not coursework, please add the NotCoursework label (and message on Slack in #cyf-curriculum or it will probably not be noticed).

If this PR needs reviewed, please add the 'Needs Review' label to this PR after you have resolved the issues listed above.

github-actions · 2025-11-20T09:43:23Z

Your PR's title isn't in the expected format.

Please check the expected title format, and update yours to match.

Reason: Wrong number of parts separated by |s

If this PR is not coursework, please add the NotCoursework label (and message on Slack in #cyf-curriculum or it will probably not be noticed).

If this PR needs reviewed, please add the 'Needs Review' label to this PR after you have resolved the issues listed above.

…ailing

github-actions · 2025-11-20T10:11:23Z

Your PR's title isn't in the expected format.

Please check the expected title format, and update yours to match.

Reason: Wrong number of parts separated by |s

If this PR is not coursework, please add the NotCoursework label (and message on Slack in #cyf-curriculum or it will probably not be noticed).

If this PR needs reviewed, please add the 'Needs Review' label to this PR after you have resolved the issues listed above.

github-actions · 2025-11-27T13:48:44Z

Your PR's title isn't in the expected format.

Please check the expected title format, and update yours to match.

Reason: Wrong number of parts separated by |s

If this PR is not coursework, please add the NotCoursework label (and message on Slack in #cyf-curriculum or it will probably not be noticed).

If this PR needs reviewed, please add the 'Needs Review' label to this PR after you have resolved the issues listed above.

LonMcGregor

Hi, This is a good idea that helps reviewers verify trainees are doing the task, and could help to visualise why we are getting trainees to practice rewriting tests to use a framework.

It looks like you have changed the reference files that trainees will start working on in the Sprint-3 directory. How much of that is intended to be committed back to main?

I am looking at the sample output here: https://github.com/CodeYourFuture/Module-Structuring-and-Testing-Data/actions/runs/19738349167

I have some thoughts:

Is there an easier way for trainees to see this output? Currently it exists in the "checks" tab, but I don't see any obvious means of directing the trainee / reviewer towards this output, other than some comments in the README.
Can trainees run this in vs code / local CLI, as the README suggests for jest on its own, so they can avoid pushing a failing PR? Could instructions be added to that effect ?
Are the tables going to be clear enough to read. I.e. are terms like "Inline Assertion Tests" and "Test Parity Check" understandable enough for trainees that may be brand new to the concept of testing?
Following from the two points above, Is there some way we could make a comment under a PR appear with a clear explanation of what needs to be changed to make it pass? This would be in line with how automated PR checks currently work.
In the linked run, the test finds issues, but github is recording the test as having passed. that should probably be fixed so it shows a cross. This might surface issues better in the github UI.

Extra question about the learning:
Do you know if currently the concept of github / CI test output is covered in a general sense, or is this the trainees first introduction to it? Does the prep for this sprint give enough background for them to understand this new kind of PR submission?

LonMcGregor · 2025-12-03T12:35:44Z

.github/workflows/test-coverage-1-implement-rewrite-tests.yml

+        continue-on-error: true
+
+      - name: Run Jest Tests with Coverage
+        run: npx jest --config=jest.config.jest-tests.js --coverage --ci


the npm test command is defined in the package.json.Is there a reason you use npx to run the same command here?

LonMcGregor · 2025-12-03T12:38:21Z

jest.config.jest-tests.js

+  coveragePathIgnorePatterns: [
+    "/node_modules/",
+  ],
+  coverageReporters: ["text", "lcov", "json-summary", "html"],


Could we reduce the coverageReporters here, or do we need all of them?

LonMcGregor · 2025-12-03T12:40:07Z

.github/workflows/test-coverage-3-stretch.yml

This file looks the same as for coverage-2. Could they be combined?

LonMcGregor · 2025-12-03T12:47:57Z

Sprint-3/1-implement-and-rewrite-tests/README.md

Test coverage runs for task 2 and task 3 in this sprint. Either the instructions should be replicated in those readmes, or a more general set of instructions should be given in the main sprint3 readme rather than here.

i786m and others added 18 commits November 8, 2025 09:12

Implemented a function getAngleType

fdc0069

Implemented a function isProperFraction

6825f69

implemented getCard Value function

be5d5ca

wrote tests for get angle type function

cad7330

wrote tests for proper fraction function

7cbd9b7

wrote tests for get card value function

bb78124

updated if condition for zero numerator

ed33a43

fix typo in comparison to if function

6678a10

Adjust coverage thresholds to 40% with 100% function coverage

f6933d2

- Set branches, lines, statements to 40% - Keep functions at 100% to ensure every function is tested - More practical for educational purposes while ensuring core functionality is covered

Simplify coverage reporting to avoid action errors

7dad219

- Remove ArtiomTr/jest-coverage-report-action (was failing) - Add simple coverage summary to GitHub Actions summary - Upload coverage artifacts with if: always() to ensure they're saved even on failure

Deploy coverage reports to GitHub Pages for browser viewing

5c265f2

- Add peaceiris/actions-gh-pages to deploy HTML reports - Coverage reports accessible at github.io URL per branch - Add coverage report link to GitHub Actions summary - No need to download artifacts, view directly in browser

Rollback to simplified ArtiomTr jest-coverage-report-action

b45e475

- Remove GitHub Pages deployment - Remove MishaKav/jest-coverage-comment - Let ArtiomTr action run tests and generate coverage itself - Simpler workflow with just inline tests + ArtiomTr coverage report

Add PR write permissions for automatic coverage comments

baaf318

- Add pull-requests: write permission to allow action to comment on PRs - ArtiomTr action will now automatically post coverage reports as PR comments - Comments will include coverage summary and file-by-file breakdown

Remove PR comment permissions

003b0ec

- Remove pull-requests: write permission - Coverage report will only show in GitHub Actions Summary - Avoids permission issues when forking to upstream repos

wheresdiasd force-pushed the refactor/implement-rewrite-coverage branch from 0cccc97 to 003b0ec Compare November 8, 2025 12:12

Refactor: Remove SH and using NodeJS / simpler approach

4c72c29

wheresdiasd force-pushed the refactor/implement-rewrite-coverage branch from 57372bb to 4c72c29 Compare November 20, 2025 08:55

Refactor: PR write permissions for job

daece44

wheresdiasd added the NotCoursework label Nov 20, 2025

Refactor: PR write permissions for job

7ffdf3b

wheresdiasd removed the NotCoursework label Nov 20, 2025

Dias, Diego added 3 commits November 20, 2025 06:06

Refactor: Added readme instructions for trainees awareness

fff9ffb

Refactor: Revert back trigger to PR

e988e4e

Refactor: Revert back trigger to PR

61e08e2

Dias, Diego added 2 commits November 20, 2025 06:36

Refactor: Tuning workflow

50fd116

Refactor: adjusting token

c15ff9e

wheresdiasd closed this Nov 20, 2025

wheresdiasd reopened this Nov 20, 2025

Dias, Diego added 4 commits November 20, 2025 06:45

Refactor: Adjusting gh actions

f07827b

Refactor: removed coverage pr comments functionality

4ac1f84

Refactor: Github actions warnings for parity issues or inline tests f…

af00810

…ailing

Refactor: Adjusting warning message

ba84bab

Dias, Diego added 3 commits November 27, 2025 10:38

Covered sprint 3 tests supporting

d6d1d7e

Covered sprint 3 tests supporting

3c050ab

File names convention

00756ef

wheresdiasd requested review from LonMcGregor, illicitonion and jennethydyrova November 27, 2025 20:30

LonMcGregor added the NotCoursework label Dec 3, 2025

LonMcGregor reviewed Dec 3, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

Refactor/implement rewrite coverage #858

Refactor/implement rewrite coverage #858

Uh oh!

wheresdiasd commented Nov 8, 2025 •

edited

Loading

Uh oh!

github-actions bot commented Nov 8, 2025

Uh oh!

github-actions bot commented Nov 20, 2025

Uh oh!

github-actions bot commented Nov 20, 2025

Uh oh!

github-actions bot commented Nov 20, 2025

Uh oh!

github-actions bot commented Nov 27, 2025

Uh oh!

LonMcGregor left a comment

Uh oh!

LonMcGregor Dec 3, 2025

Uh oh!

LonMcGregor Dec 3, 2025

Uh oh!

LonMcGregor Dec 3, 2025

Uh oh!

LonMcGregor Dec 3, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Refactor/implement rewrite coverage #858

Are you sure you want to change the base?

Refactor/implement rewrite coverage #858

Uh oh!

Conversation

wheresdiasd commented Nov 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions bot commented Nov 8, 2025

Uh oh!

github-actions bot commented Nov 20, 2025

Uh oh!

github-actions bot commented Nov 20, 2025

Uh oh!

github-actions bot commented Nov 20, 2025

Uh oh!

github-actions bot commented Nov 27, 2025

Uh oh!

LonMcGregor left a comment

Choose a reason for hiding this comment

Uh oh!

LonMcGregor Dec 3, 2025

Choose a reason for hiding this comment

Uh oh!

LonMcGregor Dec 3, 2025

Choose a reason for hiding this comment

Uh oh!

LonMcGregor Dec 3, 2025

Choose a reason for hiding this comment

Uh oh!

LonMcGregor Dec 3, 2025

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

wheresdiasd commented Nov 8, 2025 •

edited

Loading