-
Notifications
You must be signed in to change notification settings - Fork 46
Add go unit test + fix trailing slash bug #168
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.
Ahhhhh oops I'm sorry I forgot to submit my drafts
install: go get -u github.com/golang/lint/golint | ||
script: $GOPATH/bin/golint -set_exit_status | ||
install: | ||
- go get -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.
iiuc this installs all modules mentioned in import statements?
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.
iiuc, yes (and according to the logs/successful build, it works)
https://travis-ci.org/w3c/wptdashboard/builds/290183421
"appengine" | ||
"appengine/datastore" | ||
"google.golang.org/appengine" | ||
"google.golang.org/appengine/datastore" |
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.
Have you tested these locally? I have an unsubstantiated feeling that AE might not like these domains, but we'll see when we try pushing this to prod.
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.
Yeah, running locally works fine.
const resultsURL = resultsURLBase + "/" + platform + "-summary.json.gz" | ||
|
||
func TestGetResultsURL_NoTestFile(t *testing.T) { | ||
checkResult( |
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 the indentation got weirded out here
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.
Yeah - Editor settings fixed, sorry about that.
emptyTestFile := "" | ||
checkResult( | ||
t, | ||
Case { |
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.
Also might have some indentation shenanigans here
|
||
func checkResult(t *testing.T, c Case) { | ||
got := getResultsURL(c.testRun, c.testFile) | ||
if got != c.expected { |
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.
Is there no standard Golang assertion library?
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.
Not that I could find. There is https://github.com/stretchr/testify if we want to hook into it.
6d9e0a1
to
e83d63c
Compare
e83d63c
to
8768956
Compare
No description provided.