Conversation
| byte[] b2 = File.ReadAllBytes(pathO); | ||
|
|
||
| CollectionAssert.AreEqual(b1, b2); | ||
| } |
There was a problem hiding this comment.
There's a lot of repetition here, is there a way to use the game version enum and make the tests more concise?
There was a problem hiding this comment.
Good idea, thanks. I've streamlined these (1-3 only).
| CollectionAssert.AreEqual(new byte[] { 0x00, 0x00, 0x00, 0x00, 0x00, 0x00 }, level2.LevelDataChunk.Seperator); | ||
| CollectionAssert.AreEqual(TRZlib.Decompress(level.Texture32Chunk.CompressedChunk), TRZlib.Decompress(level2.Texture32Chunk.CompressedChunk)); | ||
| CollectionAssert.AreEqual(TRZlib.Decompress(level.Texture16Chunk.CompressedChunk), TRZlib.Decompress(level2.Texture16Chunk.CompressedChunk)); | ||
| CollectionAssert.AreEqual(TRZlib.Decompress(level.SkyAndFont32Chunk.CompressedChunk), TRZlib.Decompress(level2.SkyAndFont32Chunk.CompressedChunk)); |
There was a problem hiding this comment.
This specialization seems to be violating inversion of control rule, maybe it's better to create a comparator for levels of concrete engine?
There was a problem hiding this comment.
Yes, agreed - it's not ideal at the moment. This is a stopgap for the time being - we have an upgraded reader/writer library in the planning and it will come with improved tests where we will compare precisely the zipped/unzipped data (I have this in a separate project which we're going to be slowly migrating over to, as it's far too big a change to drop in one go). The TR4/5 IO is currently unused as it stands, so this will be a future task
| writer.WriteLevelToFile(level, path); | ||
| return reader.ReadLevel(path); | ||
| } | ||
| } |
There was a problem hiding this comment.
Can we use in-memory file buffers for this rather than temporary files?
There was a problem hiding this comment.
I've added a comment to remind myself to add support for this in the updated IO library - agree it will be a better approach for testing and indeed may be useful elsewhere.
| public void ReadWriteStronghold() | ||
| { | ||
| ReadWriteTR1Level(TRLevelNames.STRONGHOLD); | ||
| } |
There was a problem hiding this comment.
Could these benefit from test parameterization?
There was a problem hiding this comment.
Great suggestion, thanks. I've added this in.
| public void ReadWriteVegas() | ||
| { | ||
| ReadWriteTR2Level(TR2LevelNames.VEGAS); | ||
| } |
| => GetTR5Level("TEST1.TRC"); | ||
|
|
||
| public static TR5Level GetTR5AltTestLevel() | ||
| => GetTR5Level("TEST2.TRC"); |
There was a problem hiding this comment.
These methods don't seem to be used anywhere?
There was a problem hiding this comment.
Ah yes, these will allow tests to run in GitHub actions, so we'll make custom levels to match up (we don't want to upload OG files for obvious reasons). Probably shouldn't have added these just yet, but they will be getting used soon, so we may as well keep them for now.
2 participants
Some refactoring of the current tests in preparation for expanding on these later. I have added an
OriginalIOcategory for anything that uses OG level files, so we can continue to exclude these from GitHub actions. In the future we will test more without needing level files, and for other cases we will use our own sample levels built in Tomb Editor.For now, the common functionality has been moved to a base class so the IO work isn't repeated. We also now include the gym, cutscenes and Gold level files in the tests. The other tests (FD, boxes/zones etc) will be getting merged into the level IO in the future so these remain mostly untouched, just some formatting updates.
Each of the current tests is passing locally:
Resolves #500.