📝 [Proposal]: Add TestConfig to app.Test() for configurable testing

[grivera64]

Feature Proposal Description

This issue proposes to add a TestWithInterrupt method to the App struct for internal testing.
This issue proposed to add a TestConfig struct as an optional parameter to app.Test() for internal testing.

Currently, app.Test() allows us to test whether an endpoint completely returns before the given timeout. If it doesn't, any provided reponse is discarded and an error only returns.

TestWithInterrupt aims to provide a way to not discard the response. In most currently available cases, there would be an EOF error (which would be returned by TestWithInterrupt as an expected behavior).
The TestConfig struct aims to provide a way to tell app.Test() to not discard the response (via the new field "ErrOnTimeout"). It would look like the following:

type TestConfig struct {
    Timeout      time.Duration
    ErrOnTimeout bool
}

In most currently available cases, there would be an EOF error (which would be returned as an expected behavior).

This method though is useful for buffered streaming that is allowed via fasthttp. This feature is coming to Fiber through the following issue and pull request:

Issue:
#3127

PR:
#3131

Alignment with Express API

N/A as it is a test feature, that is similar to app.Test.

HTTP RFC Standards Compliance

N/A as it is a test feature, that is similar to app.Test.

API Stability

This would be a new method, keeping the current app.Test the same. This will help avoid changes to current tests.

Feature Examples

app := New()
app.Get("/", func(c Ctx) error {
	time.Sleep(1 * time.Second)
	return c.SendString("Should never be called")
})

respBody, err := app.Test(httptest.NewRequest(MethodGet, "/", nil), &TestConfig{
    Timeout: 100*time.Millisecond,
    ErrOnTimeout: false,  // If false, do not discard response
})

Checklist:

• I agree to follow Fiber's Code of Conduct.
• I have searched for existing issues that describe my proposal before opening this one.
• I understand that a proposal that does not meet these guidelines may be closed without explanation.

[gaby]

@grivera64 Couldnt this be a param in app.Test() ?

[grivera64]

@grivera64 Couldnt this be a param in app.Test() ?

Adding this as a param to the existing app.Test() method would require refactoring previous uses of it. To avoid this, I had proposed to make a separate method for it.

As it is a major update (v2->v3), this could be a permissible breaking change if it would make more sense to keep it as one method in the long run.

If it were to be the same method, we could change:

func (app *App) Test(req *http.Request, timeout ...time.Duration) (*http.Response, error)

to something like:

func (app *App) Test(req *http.Request, errOnTimeout bool, timeout ...time.Duration) (*http.Response, error)

Or:

func (app *App) Test(req *http.Request, config ...*TestConfig) (*http.Response, error)

Where TestConfig is a structure that has the errOnTimeout and timeout parameters as public fields.

Was this what you were referring to @gaby ?

[efectn]

aims

I think func (app *App) Test(req *http.Request, config ...*TestConfig) (*http.Response, error) is OK

[grivera64]

aims

I think func (app *App) Test(req *http.Request, config ...*TestConfig) (*http.Response, error) is OK

@efectn For sure, this will make it cleaner to indicate default vs customized behavior.

I think all we need to do is define the default behavior and this is ready to be worked on. I think a good default would be:

config := &TestConfig{
    Timeout: -1,
    ErrOnTimeout: true,
}

I will modify the issue and get working on a PR for this.

[gaby]

@grivera64 Timeout should the same it's now, I believe that's 1 sec?

[gaby]

Yes, default is 1 sec.

https://github.com/gofiber/fiber/blob/main/app.go#L871

[grivera64]

@grivera64 Timeout should the same it's now, I believe that's 1 sec?

You are right, my mistake. The default to match current functionality would rather be:

config := &TestConfig{
    Timeout: time.Second,
    ErrOnTimeout: true,
}

Thanks for catching me on that, @gaby !

[gaby]

@grivera64 Do you want to be assigned this issue?