Tests for MethodNotAllowed error

[rodionovv]

What type of PR is this? (check all applicable)

• Refactor
• Feature
• Bug Fix
• Optimization
• Documentation Update
• Go Version Update
• Dependency Update

Description

Hello!

This pr adds some tests for checking that MethodNotAllowed error is returned correctly in some cases. All new tests are passing in current main branch state. But some of them are not passing in v1.8.1 (b4617d0).

In released version 404 can be returned instead of 405 when after adding required route, another one with different method and url was added. And also there is a difference in how the route was created. It is also important to make a request to required path with Method used in the second route. Sending request with every new and different method that was added after required route with required method will return 404 instead of 405 . Sending requests with method that hasn't been previously added will work fine and return 405. Example:

func TestMethodNotAllowed_diffMethods_bothHandleFuncs(t *testing.T) {
	handler := func(w http.ResponseWriter, r *http.Request) { w.WriteHeader(http.StatusOK) }
	router := NewRouter()

	router.HandleFunc("/thing", handler).Methods(http.MethodPost)
	router.HandleFunc("/something", handler).Methods(http.MethodGet)

	w := NewRecorder()
	req := newRequest(http.MethodGet, "/thing")

	router.ServeHTTP(w, req)

	if w.Code != http.StatusMethodNotAllowed {
		t.Fatalf("Expected status code 405 (got %d)", w.Code)
	}
}

This test is passing in both versions. But next example is not passing in v1.18.1:

func TestMethodNotAllowed_diffMethods_handleFuncMethods(t *testing.T) {
	handler := func(w http.ResponseWriter, r *http.Request) { w.WriteHeader(http.StatusOK) }
	router := NewRouter()

	router.HandleFunc("/thing", handler).Methods(http.MethodPost)
	router.Methods(http.MethodGet).Path("/something").Handler(http.HandlerFunc(handler))

	w := NewRecorder()
	req := newRequest(http.MethodGet, "/thing")

	router.ServeHTTP(w, req)

	if w.Code != http.StatusMethodNotAllowed {
		t.Fatalf("Expected status code 405 (got %d)", w.Code)
	}
}

The only change here is that HandleFunc().Methods() is replaced with Methods().Path().Handler(). So different way of creation of router leads to an error. The difference in v1.18.1 in produced routes was in order of matchers in inner array. Thats want I've mentioned. Wasn't able to identify how this influenced the result. Also I found different ways of how routes can be produced with this error:

router.Methods(http.MethodPost).Path("/thing").Handler(http.HandlerFunc(handler))
router.Methods(http.MethodGet).Path("/something").Handler(http.HandlerFunc(handler)) 

router.HandleFunc("/thing", handler).Methods(http.MethodPost)
router.Methods(http.MethodGet).Path("/something").Handler(http.HandlerFunc(handler))

router.Path("/thing").Methods(http.MethodPost).Handler(http.HandlerFunc(handler))
router.Methods(http.MethodGet).Path("/something").Handler(http.HandlerFunc(handler)) 

Currently provided tests do not cover cases of route possible varying way of creation except HandleFunc . So i added them, trying to check all possible ways of adding routes.

Despite issue being fixed new tests can ensure that it wont return later.

If I've missed something, let me know please.

Related Tickets & Documents

• Related Issue #
• Closes #

Added/updated tests?

• Yes
• No, and this is why: please replace this line with details on why tests
  have not been included
• I need help with writing tests

Run verifications and test

• make verify is passing
• make test is passing