add generator commit hash to node metadata #53
Conversation
main_test.go
Outdated
| func TestCommitShaIdInNodeMetadata(t *testing.T) { | ||
|
|
||
| // Build td-grpc-bootstrap binary. | ||
| cmd := exec.Command("go", "build", ".") |
There was a problem hiding this comment.
Oh, gosh, no. This is asking to be super confusing. Nobody expects a test to run go build.
Strictly better would be to run the tests with -buildvcs=true and call getGitCommitId(). Then just make sure it isn't an error and it looks like a SHA.
There was a problem hiding this comment.
:') I see your point. Updating this in the next rev.
We discussed something similar a while ago about how to get test the generator for its default behavior with flags. One of the testing methods you suggested was to do something similar to running go build and executing the binary. I was thinking all of our tests should look like this.
Not for this change tho:
Do you think maybe adding a build and generate test to the github workflow might be worth it? And compare that to a golden file?
There was a problem hiding this comment.
I don't think I suggested go build. I might have suggested running the test binary though, like done in Go's CGI tests.
main_test.go
Outdated
|
|
||
| re := regexp.MustCompile(`^[a-f0-9]{40}$`) | ||
| if !re.MatchString(commitId) { | ||
| t.Fatal("getGitCommitId(): returned an invalid commit ID. Want commit ID to be a valid SHA1 hash.") |
There was a problem hiding this comment.
Include the commitId in the message?
For some of the features in flight, it would be nice to determine the version of the generator used by looking at the Node metadata that is being passed to TD. This change add a new value to Node.Metadata keyed with
TRAFFICDIRECTOR_GRPC_BOOTSTRAP_GENERATOR_SHAand is plumbed the git commit ID at HEAD.