mirror of
https://github.com/fnproject/fn.git
synced 2022-10-28 21:29:17 +03:00
remove flaky tests (#972)
if we want them back, we can dig them out of git instead of some poor soul uncommenting them 10 years from now and spending 3 months on failing CI builds trying to figure out how a test that breaks doesn't mean the code's broke. these tests are notoriously flaky and hard to understand/fix, they also test very specific agent behaviors all the way through the front end when it may be easier to test them in unit tests instead (should we so choose). at least, since the behaviors tested aren't being changed very often, these are only serving to provide negative value in time wasted re-running the test suite [since them failing doesn't really indicate the code being wrong]. the `IOPipes` test is partially covered by `TestPipesAreClear` which hasn't cropped up as being as flaky, but it tests less behaviors. it is not easy tt o understand, either. while i think we learned a lot from these tests, they haven't been a great citizen of our test suite at large, i figure if we need to change runner behavior in the future we can maybe make another go at it.
This commit is contained in:
@@ -11,7 +11,6 @@ import (
|
||||
"os"
|
||||
"strings"
|
||||
"testing"
|
||||
"time"
|
||||
|
||||
"github.com/fnproject/fn/api/agent"
|
||||
"github.com/fnproject/fn/api/datastore"
|
||||
@@ -146,270 +145,6 @@ func TestRouteRunnerPost(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
func TestRouteRunnerFastFail(t *testing.T) {
|
||||
buf := setLogBuffer()
|
||||
isFailure := false
|
||||
|
||||
tweaker1 := envTweaker("FN_MAX_TOTAL_MEMORY_BYTES", "134217728") // 128MB
|
||||
tweaker2 := envTweaker("FN_ENABLE_NB_RESOURCE_TRACKER", "yes") // enable fast-fail (no wait on CPU/Mem)
|
||||
defer tweaker1()
|
||||
defer tweaker2()
|
||||
|
||||
// Log once after we are done, flow of events are important (hot/cold containers, idle timeout, etc.)
|
||||
// for figuring out why things failed.
|
||||
defer func() {
|
||||
if isFailure {
|
||||
t.Log(buf.String())
|
||||
}
|
||||
}()
|
||||
|
||||
rCfg := map[string]string{"ENABLE_HEADER": "yes", "ENABLE_FOOTER": "yes"} // enable container start/end header/footer
|
||||
rImg := "fnproject/fn-test-utils"
|
||||
|
||||
app := &models.App{Name: "foo"}
|
||||
app.SetDefaults()
|
||||
|
||||
ds := datastore.NewMockInit(
|
||||
[]*models.App{app},
|
||||
[]*models.Route{
|
||||
{Path: "/json", AppID: app.ID, Image: rImg, Type: "sync", Format: "json", Memory: 70, Timeout: 30, IdleTimeout: 30, Config: rCfg},
|
||||
},
|
||||
)
|
||||
|
||||
rnr, cancelrnr := testRunner(t, ds)
|
||||
defer cancelrnr()
|
||||
|
||||
srv := testServer(ds, &mqs.Mock{}, ds, rnr, ServerTypeFull)
|
||||
ok := `{"sleepTime": 1000, "isDebug": true}`
|
||||
|
||||
results := make(chan error)
|
||||
|
||||
type tester struct {
|
||||
path string
|
||||
body string
|
||||
method string
|
||||
expectedCode int
|
||||
expectedErrSubStr string
|
||||
}
|
||||
|
||||
for idx, test := range []tester{
|
||||
{"/r/foo/json/", ok, "GET", http.StatusOK, ""},
|
||||
{"/r/foo/json/", ok, "GET", http.StatusOK, ""},
|
||||
{"/r/foo/json/", ok, "GET", http.StatusOK, ""},
|
||||
{"/r/foo/json/", ok, "GET", http.StatusOK, ""},
|
||||
} {
|
||||
go func(i int, test tester) {
|
||||
body := strings.NewReader(test.body)
|
||||
_, rec := routerRequest(t, srv.Router, test.method, test.path, body)
|
||||
respBytes, _ := ioutil.ReadAll(rec.Body)
|
||||
respBody := string(respBytes)
|
||||
maxBody := len(respBody)
|
||||
if maxBody > 1024 {
|
||||
maxBody = 1024
|
||||
}
|
||||
|
||||
if rec.Code != test.expectedCode {
|
||||
results <- fmt.Errorf("Test %d: Expected status code to be %d but was %d. body: %s",
|
||||
i, test.expectedCode, rec.Code, respBody[:maxBody])
|
||||
} else if test.expectedErrSubStr != "" && !strings.Contains(respBody, test.expectedErrSubStr) {
|
||||
results <- fmt.Errorf("Test %d: Expected response to include %s but got body: %s",
|
||||
i, test.expectedErrSubStr, respBody[:maxBody])
|
||||
} else {
|
||||
results <- nil
|
||||
}
|
||||
|
||||
}(idx, test)
|
||||
}
|
||||
|
||||
totalSuccess := 0
|
||||
totalFail := 0
|
||||
|
||||
// Scan for 4 test results
|
||||
for i := 0; i < 4; i++ {
|
||||
err := <-results
|
||||
if err != nil {
|
||||
t.Logf("Test %d: received: %s (this is probably OK)", i, err.Error())
|
||||
totalFail++
|
||||
} else {
|
||||
totalSuccess++
|
||||
}
|
||||
}
|
||||
|
||||
if totalSuccess != 1 {
|
||||
t.Errorf("Expected 1 success but got %d (fail: %d)", totalSuccess, totalFail)
|
||||
isFailure = true
|
||||
}
|
||||
}
|
||||
|
||||
func TestRouteRunnerIOPipes(t *testing.T) {
|
||||
buf := setLogBuffer()
|
||||
isFailure := false
|
||||
|
||||
// let's make freezer immediate, so that we don't deal with
|
||||
// more timing related issues below. Slightly gains us a bit more
|
||||
// determinism.
|
||||
tweaker1 := envTweaker("FN_FREEZE_IDLE_MSECS", "0")
|
||||
tweaker2 := envTweaker("FN_MAX_LOG_SIZE_BYTES", "5")
|
||||
defer tweaker1()
|
||||
defer tweaker2()
|
||||
|
||||
// Log once after we are done, flow of events are important (hot/cold containers, idle timeout, etc.)
|
||||
// for figuring out why things failed.
|
||||
defer func() {
|
||||
if isFailure {
|
||||
t.Log(buf.String())
|
||||
}
|
||||
}()
|
||||
|
||||
rCfg := map[string]string{"ENABLE_HEADER": "yes", "ENABLE_FOOTER": "yes"} // enable container start/end header/footer
|
||||
rImg := "fnproject/fn-test-utils"
|
||||
|
||||
app := &models.App{Name: "zoo"}
|
||||
app.SetDefaults()
|
||||
|
||||
ds := datastore.NewMockInit(
|
||||
[]*models.App{app},
|
||||
[]*models.Route{
|
||||
{Path: "/json", AppID: app.ID, Image: rImg, Type: "sync", Format: "json", Memory: 64, Timeout: 30, IdleTimeout: 30, Config: rCfg},
|
||||
{Path: "/http", AppID: app.ID, Image: rImg, Type: "sync", Format: "http", Memory: 64, Timeout: 30, IdleTimeout: 30, Config: rCfg},
|
||||
},
|
||||
)
|
||||
|
||||
rnr, cancelrnr := testRunner(t, ds)
|
||||
defer cancelrnr()
|
||||
|
||||
srv := testServer(ds, &mqs.Mock{}, ds, rnr, ServerTypeFull)
|
||||
|
||||
// sleep between logs and with debug enabled, fn-test-utils will log header/footer below:
|
||||
immediateGarbage := `{"isDebug": true, "postOutGarbage": "YOGURT_YOGURT_YOGURT", "postSleepTime": 0}`
|
||||
immediateJsonValidGarbage := `{"isDebug": true, "postOutGarbage": "\r", "postSleepTime": 0}`
|
||||
delayedGarbage := `{"isDebug": true, "postOutGarbage": "YOGURT_YOGURT_YOGURT", "postSleepTime": 1500}`
|
||||
ok := `{"isDebug": true}`
|
||||
|
||||
containerIds := make([]string, 0)
|
||||
|
||||
testCases := []struct {
|
||||
path string
|
||||
body string
|
||||
method string
|
||||
expectedCode int
|
||||
expectedErrSubStr string
|
||||
expectedLogsSubStr []string
|
||||
sleepAmount time.Duration
|
||||
}{
|
||||
//
|
||||
// JSON WORLD
|
||||
//
|
||||
// this should go through.
|
||||
{"/r/zoo/json/", immediateJsonValidGarbage, "GET", http.StatusOK, "", nil, 0},
|
||||
|
||||
// CASE I: immediate garbage: likely to be in the json decoder buffer after json resp parsing
|
||||
{"/r/zoo/json/", immediateGarbage, "GET", http.StatusOK, "", nil, 0},
|
||||
|
||||
// CASE II: delayed garbage: make sure delayed output lands in between request processing, should be blocked until next req
|
||||
{"/r/zoo/json/", delayedGarbage, "GET", http.StatusOK, "", nil, time.Millisecond * 2500},
|
||||
|
||||
// CASE III: normal, but should get faulty I/O from previous
|
||||
{"/r/zoo/json/", ok, "GET", http.StatusBadGateway, "invalid json", nil, 0},
|
||||
|
||||
// CASE IV: should land on CASE III container
|
||||
{"/r/zoo/json/", ok, "GET", http.StatusOK, "", nil, 0},
|
||||
|
||||
//
|
||||
// HTTP WORLD
|
||||
//
|
||||
// CASE I: immediate garbage: should be ignored (TODO: this should test immediateGarbage case, FIX THIS)
|
||||
{"/r/zoo/http", ok, "GET", http.StatusOK, "", nil, 0},
|
||||
|
||||
// CASE II: delayed garbage: make sure delayed output lands in between request processing, freezer should block,
|
||||
// bad IO lands on next request.
|
||||
{"/r/zoo/http", delayedGarbage, "GET", http.StatusOK, "", nil, time.Second * 2},
|
||||
|
||||
// CASE III: normal, but should not land on any container from case I/II.
|
||||
{"/r/zoo/http/", ok, "GET", http.StatusBadGateway, "invalid http", nil, 0},
|
||||
|
||||
// CASE IV: should land on CASE III container
|
||||
{"/r/zoo/http/", ok, "GET", http.StatusOK, "", nil, 0},
|
||||
}
|
||||
|
||||
callIds := make([]string, len(testCases))
|
||||
|
||||
for i, test := range testCases {
|
||||
body := strings.NewReader(test.body)
|
||||
_, rec := routerRequest(t, srv.Router, test.method, test.path, body)
|
||||
respBytes, _ := ioutil.ReadAll(rec.Body)
|
||||
respBody := string(respBytes)
|
||||
maxBody := len(respBody)
|
||||
if maxBody > 1024 {
|
||||
maxBody = 1024
|
||||
}
|
||||
|
||||
containerIds = append(containerIds, "N/A")
|
||||
callIds[i] = rec.Header().Get("Fn_call_id")
|
||||
|
||||
if rec.Code != test.expectedCode {
|
||||
isFailure = true
|
||||
t.Errorf("Test %d: Expected status code to be %d but was %d. body: %s",
|
||||
i, test.expectedCode, rec.Code, respBody[:maxBody])
|
||||
}
|
||||
|
||||
if test.expectedErrSubStr != "" && !strings.Contains(respBody, test.expectedErrSubStr) {
|
||||
isFailure = true
|
||||
t.Errorf("Test %d: Expected response to include %s but got body: %s",
|
||||
i, test.expectedErrSubStr, respBody[:maxBody])
|
||||
|
||||
}
|
||||
|
||||
if rec.Code == http.StatusOK {
|
||||
dockerId, err := getDockerId(respBytes)
|
||||
if err != nil {
|
||||
isFailure = true
|
||||
t.Errorf("Test %d: cannot fetch docker id body: %s",
|
||||
i, respBody[:maxBody])
|
||||
}
|
||||
containerIds[i] = dockerId
|
||||
}
|
||||
|
||||
t.Logf("Test %d: dockerId: %v", i, containerIds[i])
|
||||
time.Sleep(test.sleepAmount)
|
||||
}
|
||||
|
||||
for i, test := range testCases {
|
||||
if test.expectedLogsSubStr != nil {
|
||||
if !checkLogs(t, i, ds, callIds[i], test.expectedLogsSubStr) {
|
||||
isFailure = true
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
jsonIds := containerIds[0:5]
|
||||
|
||||
// now cross check JSON container ids:
|
||||
if jsonIds[1] != jsonIds[2] &&
|
||||
jsonIds[0] == jsonIds[1] &&
|
||||
jsonIds[3] == "N/A" &&
|
||||
jsonIds[2] != jsonIds[3] &&
|
||||
jsonIds[3] != jsonIds[4] {
|
||||
t.Logf("json container ids are OK, ids=%v", jsonIds)
|
||||
} else {
|
||||
isFailure = true
|
||||
t.Errorf("json container ids are not OK, ids=%v", jsonIds)
|
||||
}
|
||||
|
||||
httpids := containerIds[5:]
|
||||
|
||||
// now cross check HTTP container ids:
|
||||
if httpids[0] == httpids[1] &&
|
||||
httpids[2] == "N/A" &&
|
||||
httpids[1] != httpids[2] &&
|
||||
httpids[2] != httpids[3] {
|
||||
t.Logf("http container ids are OK, ids=%v", httpids)
|
||||
} else {
|
||||
isFailure = true
|
||||
t.Errorf("http container ids are not OK, ids=%v", httpids)
|
||||
}
|
||||
}
|
||||
|
||||
func TestRouteRunnerExecEmptyBody(t *testing.T) {
|
||||
buf := setLogBuffer()
|
||||
isFailure := false
|
||||
|
||||
Reference in New Issue
Block a user