Skip to content

Conversation

@zimeg
Copy link
Member

@zimeg zimeg commented Aug 2, 2025

Summary

This PR refactors logic and tests to use ESM for build problems found in a downstream prerelease using the changes of both #104 and #105 🤖

🔗 slackapi/slack-github-action#484 (comment)

Requirements

@zimeg zimeg self-assigned this Aug 2, 2025
@zimeg zimeg added javascript Pull requests that update Javascript code semver:patch addressing/merging the change would necessitate a patch semver release labels Aug 2, 2025
@codecov
Copy link

codecov bot commented Aug 2, 2025

Codecov Report

❌ Patch coverage is 91.30435% with 8 lines in your changes missing coverage. Please review.
✅ Project coverage is 62.43%. Comparing base (be9558e) to head (417e6e1).

Files with missing lines Patch % Lines
src/report.js 0.00% 4 Missing ⚠️
src/get-sha.js 0.00% 2 Missing ⚠️
src/index.js 0.00% 2 Missing ⚠️
Additional details and impacted files
@@                     Coverage Diff                      @@
##           zimeg-feat-spell-checks     #108       +/-   ##
============================================================
- Coverage                    94.66%   62.43%   -32.24%     
============================================================
  Files                            6        8        +2     
  Lines                          375      378        +3     
  Branches                         0       57       +57     
============================================================
- Hits                           355      236      -119     
- Misses                          20      142      +122     

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

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Copy link
Member Author

@zimeg zimeg left a comment

Choose a reason for hiding this comment

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

👾 Quick notes for the editor and reviewers!

"test": "npm run lint && c8 npm run test:mocha",
"test": "npm run lint && vitest run --coverage",
"test:watch": "vitest",
"test:mocha": "mocha --config .mocharc.json test/*-test.js test/**/*-test.js"
Copy link
Member Author

Choose a reason for hiding this comment

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

Suggested change
"test:mocha": "mocha --config .mocharc.json test/*-test.js test/**/*-test.js"

🪓 chore: Update these scripts

Comment on lines +3 to +14
// Mock the dependencies first
vi.mock('../src/score_components/find-problematic-comments.js', () => ({
default: vi.fn(),
}));

vi.mock('../src/score_components/coverage.js', () => ({
default: vi.fn(),
}));

vi.mock('../src/report.js', () => ({
default: vi.fn(),
}));
Copy link
Member Author

Choose a reason for hiding this comment

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

🔍 question: Is this proper mocking or does this replace what we're testing?

Comment on lines +259 to +261

reportStatus.mockResolvedValue(-205);

Copy link
Member Author

Choose a reason for hiding this comment

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

📣 question: Hmm...?

fakeCore.getInput.mockImplementation((input) => {
if (input === 'codecov_token') return 'abcd1234';
if (input === 'codecov_max_attempts') return '2';
if (input === 'codecov_retry_delay') return '12';
Copy link
Member Author

Choose a reason for hiding this comment

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

🔍 note: This was changed to avoid edge cases in timing ranges.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

javascript Pull requests that update Javascript code semver:patch addressing/merging the change would necessitate a patch semver release

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants