ronantakizawa/codereview-bench huggingface dataset:
Repository: fastify/fastify
File: test/noop-set.test.js (JavaScript)

Code before:
'use strict'

const { test } = require('node:test')
const noopSet = require('../lib/noop-set')

test('does a lot of nothing', async t => {
  const aSet = noopSet()
  t.assert.ok(aSet, 'object')

  const item = {}
  aSet.add(item)
  aSet.add({ another: 'item' })
  aSet.delete(item)
  t.assert.strictEqual(aSet.has(item), true)

  for (const i of aSet) {
    t.assert.fail('should not have any items' + i)
  }
})


Review comment (bug):
```suggestion
    t.assert.fail('should not have any items: ' + i)
```

Code after:
'use strict'

const { test } = require('node:test')
const noopSet = require('../lib/noop-set')

test('does a lot of nothing', async t => {
  const aSet = noopSet()
  t.assert.ok(aSet, 'object')

  const item = {}
  aSet.add(item)
  aSet.add({ another: 'item' })
  aSet.delete(item)
  t.assert.strictEqual(aSet.has(item), true)

  for (const i of aSet) {
    t.assert.fail('should not have any items: ' + i)
  }
})
Repository: fastify/fastify
File: test/custom-parser.5.test.js (JavaScript)

Code before:
const { getServerUrl, plainTextParser } = require('./helper')

process.removeAllListeners('warning')

test('cannot remove all content type parsers after binding', (t, done) => {
  t.plan(2)

  const fastify = Fastify()

  fastify.listen({ port: 0 }, function (err) {
    t.assert.ifError(err)
    t.assert.throws(() => fastify.removeAllContentTypeParsers())
    fastify.close()
    done()
  })
})

test('cannot remove content type parsers after binding', (t, done) => {
  t.plan(2)

  const fastify = Fastify()

  fastify.listen({ port: 0 }, function (err) {
    t.assert.ifError(err)
    t.assert.throws(() => fastify.removeContentTypeParser('application/json'))
    fastify.close()
    done()
  })
})

test('should be able to override the default json parser after removeAllContentTypeParsers', (t, done) => {
  t.plan(5)

  const fastify = Fastify()

  fastify.post('/', (req, reply) => {
    reply.send(req.body)
  })

  fastify.removeAllContentTypeParsers()

  fastify.addContentTypeParser('application/json', function (req, payload, done) {
    t.assert.ok('called')
    jsonParser(payload, function (err, body) {
      done(err, body)
    })
  })

  fastify.listen({ port: 0 }, err => {
    t.assert.ifError(err)


Review comment (suggestion):
in place of the `teardown`, we can use:

```js
t.after(() => fastify.close())
```

Code after:
const { getServerUrl, plainTextParser } = require('./helper')

process.removeAllListeners('warning')

test('cannot remove all content type parsers after binding', (t, done) => {
  t.plan(2)

  const fastify = Fastify()

  fastify.listen({ port: 0 }, function (err) {
    t.assert.ifError(err)
    t.assert.throws(() => fastify.removeAllContentTypeParsers())
    fastify.close()
    done()
  })
})

test('cannot remove content type parsers after binding', (t, done) => {
  t.plan(2)

  const fastify = Fastify()

  t.after(() => fastify.close())

  fastify.listen({ port: 0 }, function (err) {
    t.assert.ifError(err)
    t.assert.throws(() => fastify.removeContentTypeParser('application/json'))
    done()
  })
})

test('should be able to override the default json parser after removeAllContentTypeParsers', (t, done) => {
  t.plan(5)

  const fastify = Fastify()

  fastify.post('/', (req, reply) => {
    reply.send(req.body)
  })

  fastify.removeAllContentTypeParsers()

  fastify.addContentTypeParser('application/json', function (req, payload, done) {
    t.assert.ok('called')
    jsonParser(payload, function (err, body) {
      done(err, body)
    })
  })

  fastify.listen({ port: 0 }, err => {
    t.assert.ifError(err)
Repository: fastify/fastify
File: test/custom-parser.5.test.js (JavaScript)

Code before:
  fastify.post('/', (req, reply) => {
    reply.send(req.body)
  })

  fastify.removeAllContentTypeParsers()
  fastify.addContentTypeParser('application/jsoff', function (req, payload, done) {
    t.assert.ok('called')
    jsonParser(payload, function (err, body) {
      done(err, body)
    })
  })

  fastify.listen({ port: 0 }, err => {
    t.assert.ifError(err)

    sget({
      method: 'POST',
      url: getServerUrl(fastify),
      body: '{"hello":"world"}',
      headers: {
        'Content-Type': 'application/jsoff'
      }
    }, (err, response, body) => {
      t.assert.ifError(err)
      t.assert.strictEqual(response.statusCode, 200)
      t.assert.deepEqual(body.toString(), JSON.stringify({ hello: 'world' }))
      fastify.close()
      done()
    })
  })
})


Review comment (suggestion):
```suggestion
      t.assert.deepStrictEqual(body.toString(), JSON.stringify({ hello: 'world' }))
```

Code after:
  fastify.post('/', (req, reply) => {
    reply.send(req.body)
  })

  fastify.removeAllContentTypeParsers()
  fastify.addContentTypeParser('application/jsoff', function (req, payload, done) {
    t.assert.ok('called')
    jsonParser(payload, function (err, body) {
      done(err, body)
    })
  })

  fastify.listen({ port: 0 }, err => {
    t.assert.ifError(err)

    sget({
      method: 'POST',
      url: getServerUrl(fastify),
      body: '{"hello":"world"}',
      headers: {
        'Content-Type': 'application/jsoff'
      }
    }, (err, response, body) => {
      t.assert.ifError(err)
      t.assert.strictEqual(response.statusCode, 200)
      t.assert.deepStrictEqual(body.toString(), JSON.stringify({ hello: 'world' }))
      fastify.close()
      done()
    })
  })
})
Repository: fastify/fastify
File: test/stream-serializers.test.js (JavaScript)

Code before:
'use strict'

const { test } = require('node:test')
const Fastify = require('..')
const Reply = require('../lib/reply')

test('should serialize reply when response stream is ended', (t, done) => {
  t.plan(3)
  const stream = require('node:stream')
  const fastify = Fastify({
    logger: {
      serializers: {
        res (reply) {
          t.assert.strictEqual(reply instanceof Reply, true)
          return reply
        }
      }
    }
  })

  fastify.get('/error', function (req, reply) {
    const reallyLongStream = new stream.Readable({
      read: () => { }
    })
    reply.code(200).send(reallyLongStream)
    reply.raw.end(Buffer.from('hello\n'))
  })

  fastify.inject({
    url: '/error',
    method: 'GET'
  }, (err) => {
    t.assert.ifError(err)
    fastify.close()
    done()
  })
})


Review comment (suggestion):
Can you add this below this assertion based on https://github.com/fastify/fastify/pull/5839#issuecomment-2472725756. Also update `t.plan()` to `t.plan(4)` to validate `t.assert.ok()` is tested
```
t.assert.ok('passed')
```

Code after:
'use strict'

const { test } = require('node:test')
const Fastify = require('..')
const Reply = require('../lib/reply')

test('should serialize reply when response stream is ended', (t, done) => {
  t.plan(5)

  const stream = require('node:stream')
  const fastify = Fastify({
    logger: {
      serializers: {
        res (reply) {
          t.assert.strictEqual(reply instanceof Reply, true)
          t.assert.ok('passed')
          return reply
        }
      }
    }
  })

  fastify.get('/error', function (req, reply) {
    const reallyLongStream = new stream.Readable({
      read: () => { }
    })
    reply.code(200).send(reallyLongStream)
    reply.raw.end(Buffer.from('hello\n'))
  })

  t.after(() => fastify.close())

  fastify.inject({
    url: '/error',
    method: 'GET'
  }, (err) => {
    t.assert.ifError(err)
    done()
  })
})
Repository: fastify/fastify
File: test/sync-routes.test.js (JavaScript)

Code before:
'use strict'

const { test } = require('node:test')
const Fastify = require('..')

test('sync route', async t => {
  const app = Fastify()
  app.get('/', () => 'hello world')
  const res = await app.inject('/')
  t.assert.strictEqual(res.statusCode, 200)
  t.assert.strictEqual(res.body, 'hello world')
  app.close()
})

test('sync route return null', async t => {
  const app = Fastify()
  app.get('/', () => null)
  const res = await app.inject('/')
  t.assert.strictEqual(res.statusCode, 200)
  t.assert.strictEqual(res.body, 'null')
  app.close()
})

test('sync route, error', async t => {
  const app = Fastify()
  app.get('/', () => {
    throw new Error('kaboom')
  })
  const res = await app.inject('/')
  t.assert.strictEqual(res.statusCode, 500)
  app.close()
})


Review comment (suggestion):
we should use `t.after(() => fastify.close())` everywhere: in this PR these syntaxes are switching 😞 

Code after:
'use strict'

const { test } = require('node:test')
const Fastify = require('..')

test('sync route', async t => {
  const fastify = Fastify()
  t.after(() => fastify.close())
  fastify.get('/', () => 'hello world')
  const res = await fastify.inject('/')
  t.assert.strictEqual(res.statusCode, 200)
  t.assert.strictEqual(res.body, 'hello world')
})

test('sync route return null', async t => {
  const fastify = Fastify()
  t.after(() => fastify.close())
  fastify.get('/', () => null)
  const res = await fastify.inject('/')
  t.assert.strictEqual(res.statusCode, 200)
  t.assert.strictEqual(res.body, 'null')
})

test('sync route, error', async t => {
  const fastify = Fastify()
  t.after(() => fastify.close())
  fastify.get('/', () => {
    throw new Error('kaboom')
  })
  const res = await fastify.inject('/')
  t.assert.strictEqual(res.statusCode, 500)
})
Repository: fastify/fastify
File: test/request-id.test.js (JavaScript)

Code before:
  const REQUEST_ID = '42'

  const fastify = Fastify({
    requestIdHeader: 'my-custom-request-id'
  })

  fastify.get('/', (req, reply) => {
    t.assert.strictEqual(req.id, REQUEST_ID)
    reply.send({ id: req.id })
  })

  t.after(() => fastify.close())

  fastify.listen({ port: 0 }, (err, address) => {
    t.assert.ifError(err)

    sget({
      method: 'GET',
      url: address,
      headers: {
        'my-custom-request-id': REQUEST_ID
      }
    }, (err, response, body) => {
      t.assert.ifError(err)
      t.assert.strictEqual(body.toString(), `{"id":"${REQUEST_ID}"}`)
      fastify.close()
      done()
    })
  })
})

test('The request id header key can be customized', (t, done) => {
  t.plan(4)
  const REQUEST_ID = '42'

  const fastify = Fastify({
    requestIdHeader: 'my-custom-request-id'
  })

  fastify.get('/', (req, reply) => {
    t.assert.strictEqual(req.id, REQUEST_ID)
    reply.send({ id: req.id })
  })

  t.after(() => fastify.close())

  fastify.listen({ port: 0 }, (err, address) => {
    t.assert.ifError(err)

    sget({
      method: 'GET',

Review comment (suggestion):
t.after is already configured here

```suggestion
```

Code after:
  const REQUEST_ID = '42'

  const fastify = Fastify({
    requestIdHeader: 'my-custom-request-id'
  })

  fastify.get('/', (req, reply) => {
    t.assert.strictEqual(req.id, REQUEST_ID)
    reply.send({ id: req.id })
  })

  t.after(() => fastify.close())

  fastify.listen({ port: 0 }, (err, address) => {
    t.assert.ifError(err)

    sget({
      method: 'GET',
      url: address,
      headers: {
        'my-custom-request-id': REQUEST_ID
      }
    }, (err, response, body) => {
      t.assert.ifError(err)
      t.assert.strictEqual(body.toString(), `{"id":"${REQUEST_ID}"}`)
      done()
    })
  })
})

test('The request id header key can be customized', (t, done) => {
  t.plan(4)
  const REQUEST_ID = '42'

  const fastify = Fastify({
    requestIdHeader: 'my-custom-request-id'
  })

  fastify.get('/', (req, reply) => {
    t.assert.strictEqual(req.id, REQUEST_ID)
    reply.send({ id: req.id })
  })

  t.after(() => fastify.close())

  fastify.listen({ port: 0 }, (err, address) => {
    t.assert.ifError(err)

    sget({
      method: 'GET',
      url: address,
Repository: fastify/fastify
File: test/router-options.test.js (JavaScript)

Code before:
  t.assert.strictEqual(res.payload.toString(), 'test')
})

test('Should honor maxParamLength option', (t, done) => {
  t.plan(4)
  const fastify = Fastify({ maxParamLength: 10 })

  fastify.get('/test/:id', (req, reply) => {
    reply.send({ hello: 'world' })
  })

  fastify.inject({
    method: 'GET',
    url: '/test/123456789'
  }, (error, res) => {
    t.assert.ifError(error)
    t.assert.strictEqual(res.statusCode, 200)
  })

  fastify.inject({
    method: 'GET',
    url: '/test/123456789abcd'
  }, (error, res) => {
    t.assert.ifError(error)
    t.assert.strictEqual(res.statusCode, 404)
    done()
  })
})

test('Should expose router options via getters on request and reply', (t, done) => {
  t.plan(9)
  const fastify = Fastify()
  const expectedSchema = {
    params: {
      type: 'object',
      properties: {
        id: { type: 'integer' }
      }
    }
  }

  fastify.get('/test/:id', {
    schema: expectedSchema
  }, (req, reply) => {
    t.assert.strictEqual(reply.routeOptions.config.url, '/test/:id')
    t.assert.strictEqual(reply.routeOptions.config.method, 'GET')
    t.assert.deepStrictEqual(req.routeOptions.schema, expectedSchema)
    t.assert.strictEqual(typeof req.routeOptions.handler, 'function')
    t.assert.strictEqual(req.routeOptions.config.url, '/test/:id')
    t.assert.strictEqual(req.routeOptions.config.method, 'GET')
    t.assert.strictEqual(req.is404, false)

Review comment (suggestion):
when there are more that one `inject`, we can't use the `done()` cb in a single one because it is not guaranteed that this cb will be the last one, so the test may completes before the other assertions

Code after:
  t.assert.strictEqual(res.payload.toString(), 'test')
})

test('Should honor maxParamLength option', async (t) => {
  const fastify = Fastify({ maxParamLength: 10 })

  fastify.get('/test/:id', (req, reply) => {
    reply.send({ hello: 'world' })
  })

  const res = await fastify.inject({
    method: 'GET',
    url: '/test/123456789'
  })
  t.assert.strictEqual(res.statusCode, 200)

  const resError = await fastify.inject({
    method: 'GET',
    url: '/test/123456789abcd'
  })
  t.assert.strictEqual(resError.statusCode, 404)
})

test('Should expose router options via getters on request and reply', (t, done) => {
  t.plan(9)
  const fastify = Fastify()
  const expectedSchema = {
    params: {
      type: 'object',
      properties: {
        id: { type: 'integer' }
      }
    }
  }

  fastify.get('/test/:id', {
    schema: expectedSchema
  }, (req, reply) => {
    t.assert.strictEqual(reply.routeOptions.config.url, '/test/:id')
    t.assert.strictEqual(reply.routeOptions.config.method, 'GET')
    t.assert.deepStrictEqual(req.routeOptions.schema, expectedSchema)
    t.assert.strictEqual(typeof req.routeOptions.handler, 'function')
    t.assert.strictEqual(req.routeOptions.config.url, '/test/:id')
    t.assert.strictEqual(req.routeOptions.config.method, 'GET')
    t.assert.strictEqual(req.is404, false)
    reply.send({ hello: 'world' })
  })

  fastify.inject({
    method: 'GET',
    url: '/test/123456789'
Repository: fastify/fastify
File: test/router-options.test.js (JavaScript)

Code before:
    ignoreTrailingSlash: true,
    ignoreDuplicateSlashes: true
  })

  fastify.get('/test//test///test', (req, res) => {
    res.send('test')
  })

  let res = await fastify.inject('/test/test/test/')
  t.assert.strictEqual(res.statusCode, 200)
  t.assert.strictEqual(res.payload.toString(), 'test')

  res = await fastify.inject('/test//test///test//')
  t.assert.strictEqual(res.statusCode, 200)
  t.assert.strictEqual(res.payload.toString(), 'test')
})

test('Should honor maxParamLength option', async (t) => {
  const fastify = Fastify({ maxParamLength: 10 })

  fastify.get('/test/:id', (req, reply) => {
    if (req.params.id.length > 10) {
      reply.code(404).send({ error: 'Parameter value exceeds maximum length' })
    } else {
      reply.send({ hello: 'world' })
    }
  })

  const res = await fastify.inject({
    method: 'GET',
    url: '/test/123456789'
  })
  t.assert.strictEqual(res.statusCode, 200)

  const resError = await fastify.inject({
    method: 'GET',
    url: '/test/123456789abcd'
  })
  t.assert.strictEqual(resError.statusCode, 404)
})

test('Should expose router options via getters on request and reply', (t, done) => {
  t.plan(9)
  const fastify = Fastify()
  const expectedSchema = {
    params: {
      type: 'object',
      properties: {
        id: { type: 'integer' }
      }
    }

Review comment (bug):
the check should be performed by fastify and not by the route

```suggestion
    reply.send({ hello: 'world' })
```

If the test is failing, we have a bug here

Code after:
    ignoreTrailingSlash: true,
    ignoreDuplicateSlashes: true
  })

  fastify.get('/test//test///test', (req, res) => {
    res.send('test')
  })

  let res = await fastify.inject('/test/test/test/')
  t.assert.strictEqual(res.statusCode, 200)
  t.assert.strictEqual(res.payload.toString(), 'test')

  res = await fastify.inject('/test//test///test//')
  t.assert.strictEqual(res.statusCode, 200)
  t.assert.strictEqual(res.payload.toString(), 'test')
})

test('Should honor maxParamLength option', async (t) => {
  const fastify = Fastify({ maxParamLength: 10 })

  fastify.get('/test/:id', (req, reply) => {
    reply.send({ hello: 'world' })
  })

  const res = await fastify.inject({
    method: 'GET',
    url: '/test/123456789'
  })
  t.assert.strictEqual(res.statusCode, 200)

  const resError = await fastify.inject({
    method: 'GET',
    url: '/test/123456789abcd'
  })
  t.assert.strictEqual(resError.statusCode, 404)
})

test('Should expose router options via getters on request and reply', (t, done) => {
  t.plan(9)
  const fastify = Fastify()
  const expectedSchema = {
    params: {
      type: 'object',
      properties: {
        id: { type: 'integer' }
      }
    }
  }

  fastify.get('/test/:id', {
    schema: expectedSchema
Repository: fastify/fastify
File: test/content-parser.test.js (JavaScript)

Code before:

    const fastify = Fastify()

    const contentTypeParser = fastify[keys.kContentTypeParser]

    t.assert.ok(!contentTypeParser.remove('image/png'))
    t.assert.deepStrictEqual(contentTypeParser.customParsers.size, 2)
  })
})

test('remove all should remove all existing parsers and reset cache', t => {
  t.plan(4)

  const fastify = Fastify()
  fastify.addContentTypeParser('application/xml', first)
  fastify.addContentTypeParser(/^image\/.*/, first)

  const contentTypeParser = fastify[keys.kContentTypeParser]

  contentTypeParser.getParser('application/xml') // fill cache with one entry
  contentTypeParser.removeAll()

  t.assert.deepStrictEqual(contentTypeParser.cache.size, 0)
  t.assert.deepStrictEqual(contentTypeParser.parserList.length, 0)
  t.assert.deepStrictEqual(contentTypeParser.parserRegExpList.length, 0)
  t.assert.deepStrictEqual(Object.keys(contentTypeParser.customParsers).length, 0)
})

test('Safeguard against malicious content-type / 1', async t => {
  const badNames = Object.getOwnPropertyNames({}.__proto__) // eslint-disable-line
  t.plan(badNames.length)

  const fastify = Fastify()

  fastify.post('/', async () => {
    return 'ok'
  })

  for (const prop of badNames) {
    const response = await fastify.inject({
      method: 'POST',
      path: '/',
      headers: {
        'content-type': prop
      },
      body: ''
    })

    t.assert.deepStrictEqual(response.statusCode, 415)
  }
})

Review comment (suggestion):
testing length here i think `strictEqual` should be used
```suggestion
  t.assert.strictEqual(Object.keys(contentTypeParser.customParsers).length, 0)
```

Code after:

    const fastify = Fastify()

    const contentTypeParser = fastify[keys.kContentTypeParser]

    t.assert.ok(!contentTypeParser.remove('image/png'))
    t.assert.strictEqual(contentTypeParser.customParsers.size, 2)
  })
})

test('remove all should remove all existing parsers and reset cache', t => {
  t.plan(4)

  const fastify = Fastify()
  fastify.addContentTypeParser('application/xml', first)
  fastify.addContentTypeParser(/^image\/.*/, first)

  const contentTypeParser = fastify[keys.kContentTypeParser]

  contentTypeParser.getParser('application/xml') // fill cache with one entry
  contentTypeParser.removeAll()

  t.assert.strictEqual(contentTypeParser.cache.size, 0)
  t.assert.strictEqual(contentTypeParser.parserList.length, 0)
  t.assert.strictEqual(contentTypeParser.parserRegExpList.length, 0)
  t.assert.strictEqual(Object.keys(contentTypeParser.customParsers).length, 0)
})

test('Safeguard against malicious content-type / 1', async t => {
  const badNames = Object.getOwnPropertyNames({}.__proto__) // eslint-disable-line
  t.plan(badNames.length)

  const fastify = Fastify()

  fastify.post('/', async () => {
    return 'ok'
  })

  for (const prop of badNames) {
    const response = await fastify.inject({
      method: 'POST',
      path: '/',
      headers: {
        'content-type': prop
      },
      body: ''
    })

    t.assert.strictEqual(response.statusCode, 415)
  }
})
Repository: fastify/fastify
File: test/find-route.test.js (JavaScript)

Code before:
  t.plan(1)
  const fastify = Fastify()

  fastify.get('/artists/:artistId', {
    schema: {
      params: { artistId: { type: 'integer' } }
    },
    handler: (req, reply) => reply.send(typeof req.params.artistId)
  })

  let route = fastify.findRoute({
    method: 'GET',
    url: '/artists/:artistId'
  })

  route.params = {
    ...route.params,
    id: ':id'
  }

  route = fastify.findRoute({
    method: 'GET',
    url: '/artists/:artistId'
  })

  t.assert.deepStrictEqual(route.params.artistId, ':artistId')
})

test('findRoute should return null when when url is not passed', t => {
  t.plan(1)
  const fastify = Fastify()

  fastify.get('/artists/:artistId', {
    schema: {
      params: { artistId: { type: 'integer' } }
    },
    handler: (req, reply) => reply.send(typeof req.params.artistId)
  })

  t.assert.strictEqual(fastify.findRoute({
    method: 'POST'
  }), null)
})

test('findRoute should return null when route cannot be found due to a different path', t => {
  t.plan(1)
  const fastify = Fastify()

  fastify.get('/artists/:artistId', {
    schema: {
      params: { artistId: { type: 'integer' } }

Review comment (suggestion):
```suggestion
  t.assert.strictEqual(route.params.artistId, ':artistId')
```

Code after:
  t.plan(1)
  const fastify = Fastify()

  fastify.get('/artists/:artistId', {
    schema: {
      params: { artistId: { type: 'integer' } }
    },
    handler: (req, reply) => reply.send(typeof req.params.artistId)
  })

  let route = fastify.findRoute({
    method: 'GET',
    url: '/artists/:artistId'
  })

  route.params = {
    ...route.params,
    id: ':id'
  }

  route = fastify.findRoute({
    method: 'GET',
    url: '/artists/:artistId'
  })

  t.assert.strictEqual(route.params.artistId, ':artistId')
})

test('findRoute should return null when when url is not passed', t => {
  t.plan(1)
  const fastify = Fastify()

  fastify.get('/artists/:artistId', {
    schema: {
      params: { artistId: { type: 'integer' } }
    },
    handler: (req, reply) => reply.send(typeof req.params.artistId)
  })

  t.assert.strictEqual(fastify.findRoute({
    method: 'POST'
  }), null)
})

test('findRoute should return null when route cannot be found due to a different path', t => {
  t.plan(1)
  const fastify = Fastify()

  fastify.get('/artists/:artistId', {
    schema: {
      params: { artistId: { type: 'integer' } }