Skip to content

Add support for lowercase HTTP methods in route() method#4747

Closed
ashikkabeer wants to merge 0 commit intofastify:mainfrom
ashikkabeer:main
Closed

Add support for lowercase HTTP methods in route() method#4747
ashikkabeer wants to merge 0 commit intofastify:mainfrom
ashikkabeer:main

Conversation

@ashikkabeer
Copy link

Checklist

Copy link
Member

@jsumners jsumners left a comment

Choose a reason for hiding this comment

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

Please add unit tests to cover this.

@ashikkabeer
Copy link
Author

@jsumners I don't really know how to write unit tests. Can you help me with some information on that? I saw there are many frameworks for that. What framwork should i use ?

@Uzlopak
Copy link
Contributor

Uzlopak commented May 13, 2023

See /test/route.js

You basically have to do:

test('route - lowercase method', t => {
    t.plan(3)
      fastify.route({
        method: 'get',
        url: '/lowercase',
        handler: function (req, reply) {
          reply.send({ hello: 'world' })
        }
      })
      sget({
        method: 'GET',
        url: 'http://localhost:' + fastify.server.address().port + '/lowercase'
      }, (err, response, body) => {
        t.error(err)
        t.equal(response.statusCode, 200)
        t.same(JSON.parse(body), { hello: 'world' })
      })
  })

I didnt test it, wrote it on my mobile. But you should get the gist...

fastify.js Outdated
// otherwise we should bind it after the declaration

if (typeof options.method === 'string') {
options.method = options.method.toUpperCase();
Copy link
Member

Choose a reason for hiding this comment

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

Note that this method could be an array, so we should support method: ['get', 'POST'] too

@github-actions github-actions bot added documentation Improvements or additions to documentation github actions Github actions related plugin Identify a pr to the doc that adds a plugin. test Issue or pr related to our testing infrastructure. typescript TypeScript related labels May 14, 2023
@ashikkabeer
Copy link
Author

@Uzlopak I got this error FAIL ​ test/route.test.js 1 failed of 187 7s
✖ Cannot read property 'port' of null

@Uzlopak
Copy link
Contributor

Uzlopak commented May 14, 2023

@ashikkabeer

Sry, i saw now, that you commented. I thought, that you closed out of frustration.

I opened #4750 as a follow up PR. It was imho a little bit more complicated to implement it properly.

@ashikkabeer
Copy link
Author

I wasn't frustrated or anything. I closed it accidentally. I'm starting to do OpenSource contributions and this is my first code contribution attempt. :)

@github-actions
Copy link

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 15, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

documentation Improvements or additions to documentation github actions Github actions related plugin Identify a pr to the doc that adds a plugin. test Issue or pr related to our testing infrastructure. typescript TypeScript related

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants