Skip to content

refactor: replace nock with undici #104

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Garbee merged 11 commits into from
Mar 30, 2026
Merged

refactor: replace nock with undici #104

Show file tree
Hide file tree
Changes from 10 commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
7807127
refactor: replace minimatch with native path.extname
Garbee Mar 20, 2026
437c6af
fix: exclude dotfiles and dot-directories to match minimatch behavior
Garbee Mar 20, 2026
de68fb1
test: add assertions for dotfile and dot-directory exclusion
Garbee Mar 20, 2026
098e2b0
add debug log
Garbee Mar 20, 2026
0ddf33b
add body to debug
Garbee Mar 20, 2026
2565ee3
refactor: replace js-yaml with yaml
Garbee Mar 20, 2026
4e10ae8
refactor: replace chai with node:assert/strict
Garbee Mar 20, 2026
f413afd
refactor: replace nock with undici for HTTP request mocking
Garbee Mar 20, 2026
0b546c4
fix: remove undici from dependencies to avoid dist bloat
Garbee Mar 20, 2026
8441a84
Merge branch 'main' into garbee/replace/nock
Garbee Mar 23, 2026
d24b6d1
fix: restore global dispatcher after each test
Garbee Mar 23, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 0 additions & 2 deletions package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@
"devDependencies": {
"@eslint/js": "^9.20.0",
"@types/mocha": "^10.0.10",
"@types/nock": "^11.1.0",
"@types/node": "^24",
"@types/sinon": "^17.0.3",
"@vercel/ncc": "^0.38.3",
Expand All @@ -30,7 +29,6 @@
"husky": "^9.1.7",
"lint-staged": "^15.4.3",
"mocha": "^11.1.0",
"nock": "^14.0.1",
"prettier": "^3.4.2",
"rimraf": "^6.0.1",
"sinon": "^19.0.2",
Expand Down
38 changes: 0 additions & 38 deletions src/git.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -415,44 +415,6 @@ describe('git', () => {
assert.ok(!result.includes('test.'))
})

it('should exclude dotfiles', async () => {
Copy link
Copy Markdown
Contributor

@straker straker Mar 23, 2026

Choose a reason for hiding this comment

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

Are these tests no longer needed? They seem unrelated to the undici change

Copy link
Copy Markdown
Member Author

@Garbee Garbee Mar 23, 2026

Choose a reason for hiding this comment

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

They are duplicates. Because of how Claude did the changes internally, it basically split things into multiple histories. This PR was actually not created off the previous one (even though my state locally was to do so.)

If you expand down further, you will see the tests are still there, the corrected forms from #103 .

const mockFiles = [
{ filename: '.eslintrc.js', status: 'modified' },
{ filename: '.prettierrc.js', status: 'added' },
{ filename: 'src/app.js', status: 'added' }
]

mockOctokit.rest.repos.compareCommits.resolves({
data: { files: mockFiles }
})

const result = await getChangedFiles(token)

assert.deepEqual(result, ['src/app.js'])
assert.ok(!result.includes('.eslintrc.js'))
assert.ok(!result.includes('.prettierrc.js'))
})

it('should exclude files under dot-directories', async () => {
const mockFiles = [
{ filename: '.github/README.md', status: 'modified' },
{ filename: '.github/workflows/ci.html', status: 'added' },
{ filename: 'src/.hidden/utils.js', status: 'added' },
{ filename: 'docs/guide.md', status: 'added' }
]

mockOctokit.rest.repos.compareCommits.resolves({
data: { files: mockFiles }
})

const result = await getChangedFiles(token)

assert.deepEqual(result, ['docs/guide.md'])
assert.ok(!result.includes('.github/README.md'))
assert.ok(!result.includes('.github/workflows/ci.html'))
assert.ok(!result.includes('src/.hidden/utils.js'))
})

it('should exclude dotfiles', async () => {
const mockFiles = [
{ filename: '.eslintrc.js', status: 'modified' },
Expand Down
182 changes: 108 additions & 74 deletions src/linter.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import 'mocha'
import assert from 'node:assert/strict'
import * as sinon from 'sinon'
import * as core from '@actions/core'
import nock from 'nock'
import { MockAgent, setGlobalDispatcher, type Interceptable } from 'undici'
import { lintFiles } from './linter'
import type { LinterResponse } from './types'

Expand All @@ -13,32 +13,38 @@ describe('linter', () => {
let errorStub: sinon.SinonStub
let debugStub: sinon.SinonStub
let readFileStub: sinon.SinonStub
beforeEach(() => {
sandbox = sinon.createSandbox()

// Stub core functions
errorStub = sandbox.stub(core, 'error')
debugStub = sandbox.stub(core, 'debug')

// Stub file system
readFileStub = sandbox.stub()
sandbox.replace(require('fs'), 'readFileSync', readFileStub)

// Enable Nock
nock.disableNetConnect()
})

afterEach(() => {
sandbox.restore()
nock.cleanAll()
nock.enableNetConnect()
})

describe('lintFiles', () => {
const apiKey = 'test-api-key'
const axeLinterUrl = 'https://test-linter.com'
const linterConfig = { rules: { 'test-rule': 'error' } }

let mockAgent: MockAgent
let mockPool: Interceptable

beforeEach(() => {
sandbox = sinon.createSandbox()

// Stub core functions
errorStub = sandbox.stub(core, 'error')
debugStub = sandbox.stub(core, 'debug')

// Stub file system
readFileStub = sandbox.stub()
sandbox.replace(require('fs'), 'readFileSync', readFileStub)

// Enable mock agent
mockAgent = new MockAgent()
setGlobalDispatcher(mockAgent)
mockAgent.disableNetConnect()
mockPool = mockAgent.get(axeLinterUrl)
})

afterEach(async () => {
sandbox.restore()
await mockAgent.close()
})
Comment thread
Garbee marked this conversation as resolved.

it('should process files and return total error count', async () => {
const files = ['test.js', 'test.html']
const fileContents: Record<string, string> = {
Expand Down Expand Up @@ -83,19 +89,25 @@ describe('linter', () => {
}
}

// Setup Nock interceptors for each file
// Setup interceptors for each file
files.forEach((file) => {
nock(axeLinterUrl)
.post('/lint-source', (body: any) => {
return (
body.filename === file &&
body.source === fileContents[file] &&
JSON.stringify(body.config) === JSON.stringify(linterConfig)
)
mockPool
.intercept({
path: '/lint-source',
method: 'POST',
body: JSON.stringify({
source: fileContents[file],
filename: file,
config: linterConfig
}),
headers: {
authorization: apiKey,
'content-type': 'application/json'
}
})
.reply(200, mockResponses[file], {
headers: { 'content-type': 'application/json' }
})
.matchHeader('authorization', apiKey)
.matchHeader('content-type', 'application/json')
.reply(200, mockResponses[file])
})

const errorCount = await lintFiles(
Expand Down Expand Up @@ -146,9 +158,9 @@ describe('linter', () => {

readFileStub.withArgs('test.js', 'utf8').returns(fileContents['test.js'])

const scope = nock(axeLinterUrl)
.post('/lint-source')
.reply(200, {
mockPool.intercept({ path: '/lint-source', method: 'POST' }).reply(
200,
{
report: {
errors: [
{
Expand All @@ -160,7 +172,9 @@ describe('linter', () => {
}
]
}
})
},
{ headers: { 'content-type': 'application/json' } }
)

const errorCount = await lintFiles(
files,
Expand All @@ -170,14 +184,16 @@ describe('linter', () => {
)

assert.equal(errorCount, 1, 'should return one error for single file')
assert.strictEqual(scope.isDone(), true, 'API request should be made')
mockAgent.assertNoPendingInterceptors()
})

it('should skip empty files', async () => {
const files = ['empty.js']
readFileStub.withArgs('empty.js', 'utf8').returns(' ')

const scope = nock(axeLinterUrl).post('/lint-source').reply(200)
mockPool
.intercept({ path: '/lint-source', method: 'POST' })
.reply(200, {})

const errorCount = await lintFiles(
files,
Expand All @@ -192,9 +208,8 @@ describe('linter', () => {
true,
'should log debug message'
)
assert.strictEqual(
scope.isDone(),
false,
assert.ok(
mockAgent.pendingInterceptors().length > 0,
'no HTTP requests should be made'
)
})
Expand All @@ -205,17 +220,21 @@ describe('linter', () => {
.withArgs('error.js', 'utf8')
.returns('<div><h1>hello world</h1></div>')

const scope = nock(axeLinterUrl).post('/lint-source').reply(200, {
error: 'API Error'
})
mockPool.intercept({ path: '/lint-source', method: 'POST' }).reply(
200,
{ error: 'API Error' },
{
headers: { 'content-type': 'application/json' }
}
)

try {
await lintFiles(files, apiKey, axeLinterUrl, linterConfig)
assert.fail('should have thrown an error')
} catch (error) {
assert.ok(error instanceof Error)
assert.equal(error.message, 'API Error')
assert.strictEqual(scope.isDone(), true, 'API request should be made')
mockAgent.assertNoPendingInterceptors()
}
})

Expand All @@ -224,17 +243,18 @@ describe('linter', () => {
const fileError = new Error('ENOENT')
readFileStub.throws(fileError)

const scope = nock(axeLinterUrl).post('/lint-source').reply(200)
mockPool
.intercept({ path: '/lint-source', method: 'POST' })
.reply(200, {})

try {
await lintFiles(files, apiKey, axeLinterUrl, linterConfig)
assert.fail('should have thrown an error')
} catch (error) {
assert.ok(error instanceof Error)
assert.equal(error.message, 'ENOENT')
assert.strictEqual(
scope.isDone(),
false,
assert.ok(
mockAgent.pendingInterceptors().length > 0,
'no HTTP requests should be made'
)
}
Expand All @@ -246,17 +266,19 @@ describe('linter', () => {
.withArgs('test.js', 'utf8')
.returns('<div><h1>hello world</h1></div>')

const scope = nock(axeLinterUrl)
.post('/lint-source')
.replyWithError('Network Error')
mockPool
.intercept({ path: '/lint-source', method: 'POST' })
.replyWithError(new Error('Network Error'))

try {
await lintFiles(files, apiKey, axeLinterUrl, linterConfig)
assert.fail('should have thrown an error')
} catch (error) {
assert.ok(error instanceof Error)
assert.ok(error.message.includes('Network Error'))
assert.strictEqual(scope.isDone(), true, 'API request should be made')
assert.ok(error instanceof TypeError)
assert.equal(error.message, 'fetch failed')
const cause = (error as any).cause
assert.ok(cause instanceof Error)
assert.equal(cause.message, 'Network Error')
}
})

Expand All @@ -266,16 +288,16 @@ describe('linter', () => {
.withArgs('test.js', 'utf8')
.returns('<div><h1>hello world</h1></div>')

const scope = nock(axeLinterUrl)
.post('/lint-source')
mockPool
.intercept({ path: '/lint-source', method: 'POST' })
.reply(500, 'Internal Server Error')

try {
await lintFiles(files, apiKey, axeLinterUrl, linterConfig)
assert.fail('should have thrown an error')
} catch (error) {
assert.ok(error instanceof Error)
assert.strictEqual(scope.isDone(), true, 'API request should be made')
mockAgent.assertNoPendingInterceptors()
}
})

Expand All @@ -285,16 +307,20 @@ describe('linter', () => {
.withArgs('test.js', 'utf8')
.returns('<div><h1>hello world</h1></div>')

const scope = nock(axeLinterUrl).post('/lint-source').reply(200, {
report: 'invalid-format'
})
mockPool.intercept({ path: '/lint-source', method: 'POST' }).reply(
200,
{ report: 'invalid-format' },
{
headers: { 'content-type': 'application/json' }
}
)

try {
await lintFiles(files, apiKey, axeLinterUrl, linterConfig)
assert.fail('should have thrown an error')
} catch (error) {
assert.ok(error instanceof Error)
assert.strictEqual(scope.isDone(), true, 'API request should be made')
mockAgent.assertNoPendingInterceptors()
}
})

Expand Down Expand Up @@ -359,24 +385,32 @@ describe('linter', () => {
// Setup file read
readFileStub.withArgs('test.js', 'utf8').returns('const x = 1;')

// Setup Nock to simulate 400 response with config validation error
const scope = nock(axeLinterUrl)
.post('/lint-source', {
source: 'const x = 1;',
filename: 'test.js',
config: invalidLinterConfig
// Setup interceptor to simulate connection error
mockPool
.intercept({
path: '/lint-source',
method: 'POST',
body: JSON.stringify({
source: 'const x = 1;',
filename: 'test.js',
config: invalidLinterConfig
}),
headers: {
'content-type': 'application/json',
authorization: apiKey
}
})
.matchHeader('content-type', 'application/json')
.matchHeader('authorization', apiKey)
.replyWithError('Invalid config')
.replyWithError(new Error('Invalid config'))

try {
await lintFiles(files, apiKey, axeLinterUrl, invalidLinterConfig)
assert.fail('Should have thrown an error')
} catch (error) {
assert.ok(error instanceof Error)
assert.ok(error.message.includes('Invalid config'))
assert.strictEqual(scope.isDone(), true, 'API request should be made')
assert.ok(error instanceof TypeError)
assert.equal(error.message, 'fetch failed')
const cause = (error as any).cause
assert.ok(cause instanceof Error)
assert.equal(cause.message, 'Invalid config')
}
})
})
Expand Down
Loading
Loading