redraskal / r6-dissect

Match Replay API/CLI for Rainbow Six: Siege's Dissect (.rec) format.
MIT License
70 stars 11 forks source link

Compatibility tests #44

Closed stnokott closed 1 year ago

stnokott commented 1 year ago

Validating equality of JSON output of multiple functions relating to DissectReader:

Each test follows the same basic procedure:

  1. Define test data root directory
  2. Iterate through replay files recursively
  3. Read each replay file
  4. Read the JSON file for each replay file containing the expected data
  5. Compare unmarshalled expexted JSON data with state of DissectReader after reading the replay file (each test has a different scope/behaviour here)

There currently is a folder for Y8S1 replays, for future (or past) seasons, we can just simply add another folder, as long as each replay comes with a expected JSON of the same name. The tests will automatically pick these files up.

To enable testing for a few aspects of the output, I had to change how the JSON is (un)marshalled for some types. In most cases, this only included implementing the json.Unmarshaler interface. For one type, MatchUpdateType though, I had to also change the json.Marshaler implementation since the previous one didn't allow deriving the original value from the marshalled JSON. (see feedback.go)

Closes #38.

stnokott commented 1 year ago

With #41, whichever gets merged last, will probably have merge conflicts due to the renaming of DissectReader to Reader. I'll happily fix them in this PR though if you want to merge #41 first.

redraskal commented 1 year ago

Thanks for handling the conflicts! You can add the rec files now with the updated gitignore

stnokott commented 1 year ago

You can add the rec files now with the updated gitignore

Oh wow, didnt even notice those weren't uploaded.

I tried uploading them, but they exceed the Git size limit of 100MB. Do you think we should add something like [Git LFS}(https://git-lfs.com)?

redraskal commented 1 year ago

Yeah we could just upload them with lfs

redraskal commented 1 year ago

How do they exceed the size limit? Isn't one like usually 12mb?

redraskal commented 1 year ago

I was not thinking LFS would be necessary

redraskal commented 1 year ago

As far as I know, we should not need LFS? But, I would agree that is the way to go if needed

stnokott commented 1 year ago

Ah no nevermind, I wasn't thinking straight, I had a test testing on the unzipped replay file which then was like 130MB. I'll remove that test, should be fine to upload then.