FDK testkit initial commit #1
Conversation
|
@treeder @zootalures new repo, new PR instead of fnproject/fn#549 |
|
Can you not check in vendor dir? Makes PR's a disaster. |
|
@treeder Out of curiosity, why? |
HTTP 501 means that FDK does not support specific format, so that's not a reason to fail test.
|
@treeder So, I tried to put deps into its own commit, so, I'm sorry if that looks like a mess, but I do recommend to review by commits. |
|
From @rdallman
ideally, the code looks a certain way, but this requires humans or so-much-static-analysis-it's-not-worth-it in 2017. (i.e. want to make sure they have a 'context' variable that has config, headers on it) |
|
tiny addendum to 1) / possibly separate test than serial test:
|
formats_test.go
Outdated
| wg.Add(times) | ||
|
|
||
| for i := 0; i < times; i++ { | ||
| go func() { |
There was a problem hiding this comment.
CallMultiple should do a sequential test, not a a parallel test
ideally it would be good to have some witness that multiple events were streamed to to the same container? but I don' think there is a way to safely guarantee this.
The purpose of this test is to verify that the FDK can handle a sequence of events to the same container (i think)
utils.go
Outdated
| } | ||
|
|
||
| func Cleanup() { | ||
| ctx := context.Background() |
There was a problem hiding this comment.
Would be useful to optionally keep the app (e.g. via env) so that I can get logs for failures.
utils.go
Outdated
| EnvAsHeader(req, env) | ||
| } | ||
|
|
||
| resp, err := http.DefaultClient.Do(req) |
There was a problem hiding this comment.
Would be useful to log the app/callId here
formats_test.go
Outdated
| t.Errorf("Output assertion error.\n\tExpected: %v\n\tActual: %v", expected, output.String()) | ||
| } | ||
| if response.StatusCode != http.StatusOK { | ||
| if response.StatusCode == http.StatusNotImplemented { |
There was a problem hiding this comment.
It's not clear how an FDK that doesn't understand a specific format can indicate that via a 501 right now (as the status code has to be encoded in the specified format)
There was a problem hiding this comment.
Would it be better to let the tested formats be specified via env? :
e.g. :
// FilterTestedFormats filters a list of formats to those that are tested for this FDK, based on a comma-separated list in the "FDK_FORMATS" environment variable
func FilterTestedFormats(formats []string )[]string {
supportedFormats := os.Getenv("FDK_FORMATS")
if supportedFormats != "" {
acceptedFormats := strings.Split(supportedFormats,",")
validFormats := []string{}
for _,af := range acceptedFormats {
for _, reqF := range formats {
if reqF == af {
validFormats = append(validFormats,reqF)
}
}
}
return validFormats
}
return formats
}
formats := FilterTestedFormats([]string{"http", "json"})
helloJohnPayload := &struct {
Name string `json:"name"`
}{}
...
There was a problem hiding this comment.
Okay, minor thing, but don't have objections.
|
@treeder good news, vendor has gone (easy to review and build). |
Dockerfile
Outdated
| RUN apk --no-cache add build-base git bzr mercurial gcc | ||
| RUN go get -u github.com/golang/dep/cmd/dep | ||
| ENV D=/go/src/github.com/fnproject/fdk-testkit | ||
| ADD . $D |
There was a problem hiding this comment.
See my pull in fn core, should gopy only gopkg files, then do dep ensure --vendor-only, then do this add in a layer after that.
No description provided.