Tests for github payloads and fresh test data

[henrikrudstrom]

Wanted to make a pr to add some missing installation.id s to the some of the payloads but got sucked into a rabbit hole...

This PR contains a tests that Deserializes the test payloads and Serializes it again, effectively checking finding missing properties in the structs (and the test data).

These tests now fail for now.

I noticed that the test data is pretty old and incomplete. So i added a simple scraper that fetches all the example payloads from https://developer.github.com/v3/activity/events/types/
Most example payloads dont contain the part containing installation id:

"installation": {
    "id": 2311213,
    "node_id": "MDIzOkludGVncmF0aW9uSW5zdGFsbGF0aW9uMjMxMTIxMw=="
}

but my undestanding is that they are present in most payloads? (at least the ones I've seen). The scraper adds this to all payloads except a few selected ones.
(the test data is in ./tmp for now).

Im happy to start making the tests pass (even more happy if someone wants to help).
But want to touch base here before i spend more time on it.

Having these tests also enable us to refactor the huge payloads.go file. by creating named structs for things like Repository Owner etc. which makes it easier to create mock payloads for testing applications using this library.

to run the tests you need to install github.com/josephburnett/jd/lib
the scraper depends on github.com/gocolly/colly
to run the scaper: go run scrape/github.go

@deankarn

[henrikrudstrom]

I could not help myself so i went ahead made the tests pass and refactored the structs.
(removed 4700 lines!!! from github/payload.go)

Realize this commit is pretty huge to review. I think if this is to be merged we should look at the tests and the test data and agree that they are correct.

1. The test data is from githubs documentation. Can we assume that is up to date? is there any legacy data that is not documented?

2. I had to make some tweaks to the test data that i scraped from githubs documentation, mostly to do with marshalling of default values and adding some data to properties that were null in the examples. ill see if i can make a diff of that so we that can be reviewed

3. I renamed at least two structs, Assignee and MergedBy. This could cause breakage.

[henrikrudstrom]

Hmm. how do i update the sha hashes in github_test.go?

[deankarn]

Wow @henrikrudstrom thanks for this! it will be a bit before I'll have time to properly review, please be patient :)

sha hashes, you could print the hashes out from the code while the tests are running and replace if that's what you mean.

This is going to be a breaking change given some of the type changes to pointers, but that's ok will take this opportunity to convert to go modules and ensure all the crazy > v2 import path stuff gets updated(go modules sigh the overcomplicated reinvent the wheel attempt of a solved problem)

[henrikrudstrom]

I can understand it will take some time to review. No rush from my side, im using the my fork for now.

Jep, thats what i meant about the sha. will try that.

Regarding pointers, i wasnt sure what was the reasoning about it. I assumed the pointers to strings were was to make them nullable? at least that made the tests pass for me, otherwise the strings where marshalled to empty strings and then they would not match the original json when i compare them. Feels a bit inconsistent though, that 95% of the strings are by-value and a few of them are by-reference. We could make them all by-value and say null == "". but dont have a strong opinion on that.

Regarding pointers to structs, i wanted to propose we make all nested structs by-value, (or at least all by-reference) but at least a consistent approach, now its a bit of a random mix.