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
📊 add refresh period to monitor mw #1898
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2,9 +2,11 @@ package monitor | |
|
||
import ( | ||
"bytes" | ||
"fmt" | ||
"io/ioutil" | ||
"net/http/httptest" | ||
"testing" | ||
"time" | ||
|
||
"github.com/gofiber/fiber/v2" | ||
"github.com/gofiber/fiber/v2/utils" | ||
|
@@ -28,16 +30,36 @@ func Test_Monitor_Html(t *testing.T) { | |
|
||
app := fiber.New() | ||
|
||
// defaults | ||
app.Get("/", New()) | ||
|
||
resp, err := app.Test(httptest.NewRequest(fiber.MethodGet, "/", nil)) | ||
|
||
utils.AssertEqual(t, nil, err) | ||
utils.AssertEqual(t, 200, resp.StatusCode) | ||
utils.AssertEqual(t, fiber.MIMETextHTMLCharsetUTF8, resp.Header.Get(fiber.HeaderContentType)) | ||
utils.AssertEqual(t, fiber.MIMETextHTMLCharsetUTF8, | ||
resp.Header.Get(fiber.HeaderContentType)) | ||
buf, err := ioutil.ReadAll(resp.Body) | ||
utils.AssertEqual(t, nil, err) | ||
utils.AssertEqual(t, true, bytes.Contains(buf, []byte("<title>"+defaultTitle+"</title>"))) | ||
timeoutLine := fmt.Sprintf("setTimeout(fetchJSON, %d)", | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. please also create a test where the setting of the configuration and its modification is checked There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. extended |
||
defaultRefresh.Milliseconds()-timeoutDiff) | ||
utils.AssertEqual(t, true, bytes.Contains(buf, []byte(timeoutLine))) | ||
|
||
b, err := ioutil.ReadAll(resp.Body) | ||
// custom config | ||
conf := Config{Title: "New " + defaultTitle, Refresh: defaultRefresh + time.Second} | ||
app.Get("/custom", New(conf)) | ||
resp, err = app.Test(httptest.NewRequest(fiber.MethodGet, "/custom", nil)) | ||
|
||
utils.AssertEqual(t, nil, err) | ||
utils.AssertEqual(t, 200, resp.StatusCode) | ||
utils.AssertEqual(t, fiber.MIMETextHTMLCharsetUTF8, | ||
resp.Header.Get(fiber.HeaderContentType)) | ||
buf, err = ioutil.ReadAll(resp.Body) | ||
utils.AssertEqual(t, nil, err) | ||
utils.AssertEqual(t, true, bytes.Contains(b, []byte("<title>"+defaultTitle+"</title>"))) | ||
utils.AssertEqual(t, true, bytes.Contains(buf, []byte("<title>"+conf.Title+"</title>"))) | ||
timeoutLine = fmt.Sprintf("setTimeout(fetchJSON, %d)", | ||
conf.Refresh.Milliseconds()-timeoutDiff) | ||
utils.AssertEqual(t, true, bytes.Contains(buf, []byte(timeoutLine))) | ||
} | ||
|
||
// go test -run Test_Monitor_JSON -race | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why don't we do this below when refresh and title has been reset or contains the default values ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If a user changes default config only, like:
then these lines will return default config directly:
so default config must be prepared beforehand 😉
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why should the users do that?
Then they have not understood the concept or we should not expose the default config
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we can unexport default config, that would be easier but I think that decision should be considered for the whole project, not just for monitor mw.
Note that if something is exported to users, then it is part of the API and meant to be usable/modifiable.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Though there does not seem to be many pablic uses of
monitor.DefaultConfig
, as per Github search.