-
Notifications
You must be signed in to change notification settings - Fork 5.4k
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
[WIP] use wildcard to check test outputs #3
Conversation
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.
Looks great! Thank you! I have a few comments...
wildcard.go
Outdated
|
||
var WILDCARD = []byte("[WILDCARD]") | ||
|
||
func Wildcard(pattern []byte, text []byte) bool { |
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.
I think this would be better inside the util.go file and tests in util_test.go. And it should be lowercase (not exported).
wildcard_test.go
Outdated
return Wildcard([]byte(a), []byte(b)) | ||
} | ||
|
||
func TestWildcard(t *testing.T) { |
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.
Nice!
Move this to util_test.go
integration_test.go
Outdated
@@ -78,6 +85,7 @@ func deno(inputFn string) (actual []byte, cachedir string, err error) { | |||
var out bytes.Buffer | |||
cmd.Stdout = &out | |||
err = cmd.Run() | |||
cmd.Wait() |
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.
Yikes. Was this causing a race condition?
Edit: I think this does nothing. Run already waits.
integration_test.go
Outdated
@@ -57,10 +58,16 @@ func checkOutput(t *testing.T, outFile string) { | |||
} | |||
|
|||
actual, _, err := deno(jsFile) | |||
fmt.Println(jsFile) | |||
fmt.Println(string(actual)) |
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.
Remove
@ry there is a bug which lays down somewhere else rather than this pr, the test result shows that |
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.
I've solved the bug you were having and made some other fix ups - pushed to your branch: qti3e/deno@d26704a
I will squash it all into one commit when landing.
Thanks Parsa!
util.go
Outdated
|
||
var WILDCARD = []byte("[WILDCARD]") | ||
|
||
func wildcard(pattern []byte, text []byte) bool { |
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.
I think this should operate on strings instead of byte arrays?
My pleasure Ryan! (thanks for fixups btw!) |
No description provided.