Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add unit tests for ModLibrary #846

Open
lethal-guitar opened this issue Apr 24, 2022 · 6 comments
Open

Add unit tests for ModLibrary #846

lethal-guitar opened this issue Apr 24, 2022 · 6 comments
Labels

Comments

@lethal-guitar
Copy link
Owner

No description provided.

@naftalimurgor
Copy link
Contributor

Hello @lethal-guitar,
I would like to take on this, any useful pointers to get started?

Thank you!

@lethal-guitar
Copy link
Owner Author

lethal-guitar commented Jun 13, 2022

Hey @naftalimurgor, sure thing!

So this is about the class ModLibrary, found here:

class ModLibrary

More specifically, the member function rescan of that class. This function looks at the mods directory, lists all the subdirectories it finds, and then merges the result with the previously known list of mods. What complicates things slightly is that we want to preserve any configuration the user has made, e.g. enabling/disabling certain mods as well as ordering mods a certain way. But we also want to deal properly with mod directories being added/deleted.

Further complicating things is the data layout, which is optimized for fast reordering. Instead of storing mod path and enabled state together in a single list, these are split across two lists, with one list containing all the mod paths, and the other list pointing into the first one via indices.

Ok, and now because all of this logic is a bit tricky, it would be nice to have some unit tests for the different cases, like dir list unchanged, mods added only, mods removed only, and mods added and removed at the same time.

Ideally, these unit tests would stub out or mock the part of the code that queries the actual filesystem, and instead allow injecting a fake list of directories for testing. How exactly to do that would be part of this work, as there are various options. I haven't thought too much about this yet, but I could imagine passing in a std::function in the constructor that does the filesystem scanning, or maybe the part of rescan that works with the results of the filesystem scan could be extracted into a free function.

EDIT: Also see https://github.com/lethal-guitar/RigelEngine/wiki/Modding-support#mods-directory for further context.

@lethal-guitar
Copy link
Owner Author

Ah, I forgot to mention, you can find existing unit tests here:

https://github.com/lethal-guitar/RigelEngine/tree/master/test

So for this issue here, I would suggest adding a new file called test_mod_library.cpp and implementing the tests there.

The testing framework used is https://github.com/catchorg/Catch2

@naftalimurgor
Copy link
Contributor

naftalimurgor commented Jun 13, 2022

@lethal-guitar
Thanks, really helpful information. Will submit a PR once I set things up

@lethal-guitar
Copy link
Owner Author

Great, sounds good! Let me know if you have any questions.

@lethal-guitar lethal-guitar closed this as not planned Won't fix, can't repro, duplicate, stale Jan 13, 2024
@bencsikandrei
Copy link
Contributor

bencsikandrei commented Jan 29, 2024

and instead allow injecting a fake list of directories for testing. How exactly to do that would be part of this work, as there are various options. I haven't thought too much about this yet, but I could imagine passing in a std::function in the constructor that does the filesystem scanning, or maybe the part of rescan that works with the results of the filesystem scan could be extracted into a free function.

I'm actually looking into trying to make some part of rescan a free function, would make it much easier to test its output (as now it's just a void function) - we can then use the free function inside the mod library class

My initial effort (#919) was trying to stub out the filesystem, sadly, even if it was a fun experiment, it's probably far from enough

EDIT: actually maybe I can use the getters to check the result of rescan, TBD

@lethal-guitar lethal-guitar reopened this Jan 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants