-
Notifications
You must be signed in to change notification settings - Fork 8k
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.
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
fix(engine): missing route params for CreateTestContext (#2778) #2803
Conversation
Codecov Report
@@ Coverage Diff @@
## master #2803 +/- ##
=======================================
Coverage 98.37% 98.38%
=======================================
Files 43 43
Lines 3148 3153 +5
=======================================
+ Hits 3097 3102 +5
Misses 38 38
Partials 13 13
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
Had the same problem when trying to follow the examples. The fix you outline would work but I think at that point especially with the need to create a separate engine to get the valid maxParam value one could just use a different approach using ServeHTTP and forget CreateTestContext. Tests could be written as follows based on your example: w := httptest.NewRecorder() // At this point add the route(s) to test engine.ServeHTTP(w,r) assert.Equal(t, http.StatusOK, w.Code) Admittedly this does not use the test context at all but I wonder if using CreateTestContext is even a good idea one could say that ServeHTTP is a better approach. Like the context method this does not start the engine. |
@RoCry Please fix the conflicts. |
Is there any plan to release this soon? Or is there any workaround? |
20fb1b8
to
76b015a
Compare
we will release the feature in v1.8.2 |
This PR trying to fix the issue we met, which exact same with #2778
If you guys have better idea to fix this, just let me know.