Skip to content

Commit

Permalink
removed parallel test execution feature, support template backticks e…
Browse files Browse the repository at this point in the history
…verywhere

Having a manifest with Go template expressions, but where all expressions only use backticks, would lead to the Go template not being executed, since such a manifest was also by coincidence a perfectly valid JSON array that was executed right away.

Fixed by always executing the Go template, regardless of whether the input already is valid JSON.

Adds more apitests, runs apitests in "make all".

Removes parallel execution test feature. this never worked properly.

see #72324
  • Loading branch information
Lucas Hinderberger authored and martinrode committed Jun 6, 2024
1 parent 266c4ee commit 0a7debf
Show file tree
Hide file tree
Showing 17 changed files with 143 additions and 268 deletions.
2 changes: 1 addition & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ GOARCH ?= amd64
GIT_COMMIT_SHA ?= $(shell git rev-list -1 HEAD)
LD_FLAGS = -ldflags="-X main.buildCommit=${GIT_COMMIT_SHA}"

all: test build
all: test build apitest

deps:
go mod download github.com/clbanning/mxj
Expand Down
168 changes: 53 additions & 115 deletions api_testsuite.go
Original file line number Diff line number Diff line change
Expand Up @@ -179,7 +179,7 @@ func (ats *Suite) Run() bool {
success := true
for k, v := range ats.Tests {
child := r.NewChild(strconv.Itoa(k))
sTestSuccess := ats.parseAndRunTest(v, ats.manifestDir, ats.manifestPath, k, 1, false, child, ats.loader)
sTestSuccess := ats.parseAndRunTest(v, ats.manifestDir, ats.manifestPath, child, ats.loader)
child.Leave(sTestSuccess)
if !sTestSuccess {
success = false
Expand Down Expand Up @@ -213,7 +213,7 @@ type TestContainer struct {
Path string
}

func (ats *Suite) parseAndRunTest(v any, manifestDir, testFilePath string, k, repeatNTimes int, runParallel bool, r *report.ReportElement, rootLoader template.Loader) bool {
func (ats *Suite) parseAndRunTest(v any, manifestDir, testFilePath string, r *report.ReportElement, rootLoader template.Loader) bool {
//Init variables
// logrus.Warnf("Test %s, Prev delimiters: %#v", testFilePath, rootLoader.Delimiters)
loader := template.NewLoader(ats.datastore)
Expand All @@ -227,12 +227,6 @@ func (ats *Suite) parseAndRunTest(v any, manifestDir, testFilePath string, k, re
loader.ServerURL = serverURL
loader.OAuthClient = ats.Config.OAuthClient

isParallelPathSpec := false
switch t := v.(type) {
case string:
isParallelPathSpec = util.IsParallelPathSpec(t)
}

//Get the Manifest with @ logic
fileh, testObj, err := template.LoadManifestDataAsRawJson(v, manifestDir)
dir := filepath.Dir(fileh)
Expand All @@ -245,98 +239,71 @@ func (ats *Suite) parseAndRunTest(v any, manifestDir, testFilePath string, k, re
return false
}

//Try to directly unmarshal the manifest into testcase array
// Parse as template always
testObj, err = loader.Render(testObj, filepath.Join(manifestDir, dir), nil)
if err != nil {
r.SaveToReportLog(err.Error())
logrus.Error(fmt.Errorf("can not render template (%s): %s", testFilePath, err))
return false
}

// Build list of test cases
var testCases []json.RawMessage
err = util.Unmarshal(testObj, &testCases)
if err == nil {
d := 1
if isParallelPathSpec || runParallel {
if repeatNTimes > 1 {
logrus.Debugf("run %s parallel: repeat %d times", filepath.Base(testFilePath), repeatNTimes)
}
d = len(testCases)
}

waitCh := make(chan bool, repeatNTimes*d)
succCh := make(chan bool, repeatNTimes*len(testCases))

go func() {
for kn := 0; kn < repeatNTimes; kn++ {
for ki, v := range testCases {
waitCh <- true
go testGoRoutine(k, kn+ki*repeatNTimes, v, ats, testFilePath, manifestDir, dir, r, loader, waitCh, succCh, isParallelPathSpec || runParallel)
}
}
}()

for i := 0; i < repeatNTimes*len(testCases); i++ {
select {
case succ := <-succCh:
if succ == false {
return false
}
}
}
} else {
// We were not able unmarshal into array, so we try to unmarshal into raw message

// Get the (optional) number of repititions from the test path spec
parallelRepititions := 1
if isParallelPathSpec {
switch t := v.(type) {
case string:
parallelRepititions, _ = util.GetParallelPathSpec(t)
}
}

// Parse as template always
requestBytes, lErr := loader.Render(testObj, filepath.Join(manifestDir, dir), nil)
if lErr != nil {
r.SaveToReportLog(lErr.Error())
logrus.Error(fmt.Errorf("can not render template (%s): %s", testFilePath, lErr))
return false
}

// If objects are different, we did have a Go template, recurse one level deep
if string(requestBytes) != string(testObj) {
return ats.parseAndRunTest([]byte(requestBytes), filepath.Join(manifestDir, dir),
testFilePath, k, parallelRepititions, isParallelPathSpec, r, loader)
}

// Its a JSON at this point, assign and proceed to parse
testObj = requestBytes

if err != nil {
// Input could not be deserialized into list, try to deserialize into single object
var singleTest json.RawMessage
err = util.Unmarshal(testObj, &singleTest)
if err == nil {

//Check if is @ and if so load the test
if util.IsPathSpec(string(testObj)) {
var sS string

err := util.Unmarshal(testObj, &sS)
if err != nil {
r.SaveToReportLog(err.Error())
logrus.Error(fmt.Errorf("can not unmarshal (%s): %s", testFilePath, err))
return false
}

return ats.parseAndRunTest(sS, filepath.Join(manifestDir, dir), testFilePath, k, parallelRepititions, isParallelPathSpec, r, template.Loader{})
} else {
return ats.runSingleTest(TestContainer{CaseByte: testObj, Path: filepath.Join(manifestDir, dir)}, r, testFilePath, loader, k, runParallel)
}
} else {
if err != nil {
// Malformed json
r.SaveToReportLog(err.Error())
logrus.Error(fmt.Errorf("can not unmarshal (%s): %s", testFilePath, err))
return false
}

testCases = []json.RawMessage{singleTest}
}

// Execute test cases
for i, testCase := range testCases {
var success bool

// If testCase can be unmarshalled as string, we may have a
// reference to another test using @ notation at hand
var testCaseStr string
err = util.Unmarshal(testCase, &testCaseStr)
if err == nil && util.IsPathSpec(testCaseStr) {
// Recurse if the testCase points to another file using @ notation
success = ats.parseAndRunTest(
testCaseStr,
filepath.Join(manifestDir, dir),
testFilePath,
r,
loader,
)
} else {
// Otherwise simply run the literal test case
success = ats.runLiteralTest(
TestContainer{
CaseByte: testCase,
Path: filepath.Join(manifestDir, dir),
},
r,
testFilePath,
loader,
i,
)
}

if !success {
return false
}
}

return true
}

func (ats *Suite) runSingleTest(tc TestContainer, r *report.ReportElement, testFilePath string, loader template.Loader, k int, isParallel bool) bool {
func (ats *Suite) runLiteralTest(tc TestContainer, r *report.ReportElement, testFilePath string, loader template.Loader, k int) bool {
r.SetName(testFilePath)

var test Case
Expand All @@ -357,9 +324,6 @@ func (ats *Suite) runSingleTest(tc TestContainer, r *report.ReportElement, testF
test.dataStore = ats.datastore
test.standardHeader = ats.StandardHeader
test.standardHeaderFromStore = ats.StandardHeaderFromStore
if isParallel {
test.ContinueOnFailure = true
}
if test.LogNetwork == nil {
test.LogNetwork = &ats.Config.LogNetwork
}
Expand Down Expand Up @@ -409,29 +373,3 @@ func (ats *Suite) loadManifest() ([]byte, error) {
ats.loader = loader
return b, err
}

func testGoRoutine(k, ki int, v json.RawMessage, ats *Suite, testFilePath, manifestDir, dir string, r *report.ReportElement, loader template.Loader, waitCh, succCh chan bool, runParallel bool) {
success := false

//Check if is @ and if so load the test
switch util.IsPathSpec(string(v)) {
case true:
var sS string
err := util.Unmarshal(v, &sS)
if err != nil {
r.SaveToReportLog(err.Error())
logrus.Error(fmt.Errorf("can not unmarshal (%s): %s", testFilePath, err))
success = false
break
}
success = ats.parseAndRunTest(sS, filepath.Join(manifestDir, dir), testFilePath, k+ki, 1, runParallel, r, loader)
default:
success = ats.runSingleTest(TestContainer{CaseByte: v, Path: filepath.Join(manifestDir, dir)},
r, testFilePath, loader, ki, runParallel)
}

succCh <- success
if success {
<-waitCh
}
}
13 changes: 13 additions & 0 deletions pkg/lib/template/template_loader.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,11 @@ type delimiters struct {

var delimsRE = regexp.MustCompile(`(?m)^[\t ]*(?://|/\*)[\t ]*template-delims:[\t ]*([^\t ]+)[\t ]+([^\t\n ]+).*$`)

// Loader loads and executes a manifest template.
//
// A manifest template is a customized version of Go's text/template, plus
// custom template functions (which are initialized with the Loader's
// properties, where applicable).
type Loader struct {
datastore *datastore.Datastore
HTTPServerHost string
Expand All @@ -50,6 +55,14 @@ func NewLoader(datastore *datastore.Datastore) Loader {
return Loader{datastore: datastore}
}

// Render loads and executes a manifest template.
//
// For a description of the manifest template, refer to Loader's docstring.
//
// - tmplBytes is the manifest template, as loaded from disk.
// - rootDir is the path of the directory in which manifest resides.
// - ctx is the data passed on to the template's Execute function.
// Contrary to what convention may suggest, it is not a context.Context.
func (loader *Loader) Render(
tmplBytes []byte,
rootDir string,
Expand Down
14 changes: 7 additions & 7 deletions test/datastore/requests.json
Original file line number Diff line number Diff line change
@@ -1,29 +1,29 @@
[
{
"name": "add data thru request - 1",
"name": "add data thru {{ printf `data1.json` }} request - 1",
"request": {
"server_url": "http://localhost:9999",
"endpoint": "data1.json",
"endpoint": "{{ printf `data1.json` }}",
"method": "GET"
},
"response": {
"body": {"some": "data"}
}
},
{
"name": "add data thru request - 2",
"name": "add data thru {{ printf `data2.json` }} request - 2",
"request": {
"server_url": "http://localhost:9999",
"endpoint": "data2.json",
"endpoint": "{{ printf `data2.json` }}",
"method": "GET"
}
},
{
"name": "add data thru request - 3",
"name": "add data thru {{ printf `data1.json` }} request - 3",
"request": {
"server_url": "http://localhost:9999",
"endpoint": "data1.json",
"endpoint": "{{ printf `data1.json` }}",
"method": "GET"
}
}
]
]
1 change: 1 addition & 0 deletions test/filemarshal/dummy.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
{"foo": "{\n \"dummy\": \"data\"\n}\n"}
3 changes: 3 additions & 0 deletions test/filemarshal/dummy_template.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
{
"dummy": {{ printf `"data"` }}
}
11 changes: 11 additions & 0 deletions test/filemarshal/manifest.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
{
"http_server": {
"addr": ":9999",
"dir": "./",
"testmode": false
},
"name": "ensure a file_render to JSON marshal pipe works as expected",
"tests": [
"@requests.json"
]
}
29 changes: 29 additions & 0 deletions test/filemarshal/requests.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
[
{
"name": "file_render to marshal pipeline should pass",
"request": {
"server_url": "http://localhost:9999",
"endpoint": "dummy.txt",
"method": "GET"
},
"response": {
"body": {
"foo": {{ file_render "dummy_template.json" | marshal }}
}
}
},
{
"name": "file to marshal pipeline should fail",
"request": {
"server_url": "http://localhost:9999",
"endpoint": "dummy.txt",
"method": "GET"
},
"response": {
"body": {
"foo": {{ file "dummy_template.json" | marshal }}
}
},
"reverse_test_result": true
}
]
26 changes: 0 additions & 26 deletions test/parallel/p/check_collected_responses.json

This file was deleted.

21 changes: 0 additions & 21 deletions test/parallel/p/manifest.json

This file was deleted.

Loading

0 comments on commit 0a7debf

Please sign in to comment.