Skip to content
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

test: validate go templates with go #30

Open
wants to merge 9 commits into
base: master
Choose a base branch
from

Conversation

qvalentin
Copy link
Collaborator

@qvalentin qvalentin commented Jan 14, 2025

  • Some testnames are still ugly
  • corpus_test.go:29: template: template:2: missing value for block clause -> it is working, I found a bug ;)
  • ci

@qvalentin qvalentin changed the title Test/validate go templates with go test: validate go templates with go Jan 14, 2025
@qvalentin qvalentin force-pushed the test/validate-go-templates-with-go branch from 8db8730 to 5727350 Compare January 14, 2025 16:54
this ensures all testcases obey the gotemplate grammar
and we are compatible with the official implementation
@qvalentin qvalentin force-pushed the test/validate-go-templates-with-go branch from 39e190b to e54ee93 Compare January 15, 2025 19:14
@qvalentin qvalentin marked this pull request as ready for review January 15, 2025 19:24
@qvalentin qvalentin requested a review from baptman21 January 15, 2025 19:24
test/corpus_test.go Outdated Show resolved Hide resolved
Comment on lines 73 to 81
partsWithEmptyParts := TESTCASE_SEPERATOR.Split(content, -1)
parts := []string{}

// remove empty parts
for i := 0; i < len(partsWithEmptyParts); i++ {
if partsWithEmptyParts[i] != "" {
parts = append(parts, partsWithEmptyParts[i])
}
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
partsWithEmptyParts := TESTCASE_SEPERATOR.Split(content, -1)
parts := []string{}
// remove empty parts
for i := 0; i < len(partsWithEmptyParts); i++ {
if partsWithEmptyParts[i] != "" {
parts = append(parts, partsWithEmptyParts[i])
}
}
parts := TESTCASE_SEPERATOR.Split(content, -1)
// remove empty parts
index := 0
for _, str := range parts {
if str != "" {
parts[index] = str
index++
}
}
parts = parts[:index]

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

idk, sometimes the simplest loop is the best loop.

test/corpus_test.go Outdated Show resolved Hide resolved
test/corpus_test.go Show resolved Hide resolved
@qvalentin qvalentin force-pushed the test/validate-go-templates-with-go branch from 0c14f5f to 9ec3efb Compare January 17, 2025 19:59
Copy link
Contributor

@guilhas07 guilhas07 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Really nice addition to the project!


var (
TESTCASE_SEPERATOR = regexp.MustCompile("(?m)^(=+)$")
INPUT_OUTPUT_SEPERATOR = regexp.MustCompile("\n(---)\n")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
INPUT_OUTPUT_SEPERATOR = regexp.MustCompile("\n(---)\n")
INPUT_OUTPUT_SEPERATOR = regexp.MustCompile("\n(-+)\n")

So a test doesn't fail to parse if it includes more than 3 dashes.

Comment on lines +88 to +89
fmt.Printf("Error parsing %s: Testcase has invalid format %s\n", filename, testName)
continue
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we should just set testParts to something like this:

Suggested change
fmt.Printf("Error parsing %s: Testcase has invalid format %s\n", filename, testName)
continue
testParts = []string{"", ""}

and check in the testTemplate against input so we get a test failure from a bad formatted test.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With this change, I can see a test failure easily:
--- FAIL: TestCorpus/corpus/actions.txt:_invalid_comments (0.00s)
(although this stops failing once the regex change suggested above is applied).

Comment on lines +14 to +16
TESTCASE_SEPERATOR = regexp.MustCompile("(?m)^(=+)$")
INPUT_OUTPUT_SEPERATOR = regexp.MustCompile("\n(---)\n")
TRIM_TESTCASE_SEPERATOR = regexp.MustCompile("^(=+)\n")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
TESTCASE_SEPERATOR = regexp.MustCompile("(?m)^(=+)$")
INPUT_OUTPUT_SEPERATOR = regexp.MustCompile("\n(---)\n")
TRIM_TESTCASE_SEPERATOR = regexp.MustCompile("^(=+)\n")
TESTCASE_SEPARATOR = regexp.MustCompile("(?m)^(=+)$")
INPUT_OUTPUT_SEPARATOR = regexp.MustCompile("\n(---)\n")
TRIM_TESTCASE_SEPARATOR = regexp.MustCompile("^(=+)\n")

typo

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants