Core: Add AutoWorld.testable_worlds to be used instead of AutoWorld.world_types in generic unittests

[NewSoupVi]

This is mostly to allow us to have greater control over what unit tests get run, but I also wanted to include a way for worlds to run generic unit tests on non default options.

Here's what this PR does:

1. Add AutoWorld.testable_worlds, which includes a list of options dicts, prefilled with "default": {} for all games
2. Replace almost every instance of AutoWorld.world_types with AutoWorlds.testable_worlds in all generic unit tests
3. Register some extra test presets in The Witness, replacing the "bare WorldTestBase" tests

Exceptions:

• The benchmark you run manually
• AutoPatchRegister test
• The one test where we explicitly get A Link To The Past
• The yaml template one that mocks AutoWorldRegister.world_types

There is currently no incentive to register extra options, but my vision is that our index / the new install process that runs unit tests on worlds you're installing "rewards" worlds for defining multiple test option presets

[BadMagic100]

Love to see this (completely by accident by the way). For the sake of your sanity I hope it can be merged quickly :)

This is probably a contentious suggestion, but thinking back to the original soft disablement pr one of the goals there was to allow opting worlds out of certain tests to prevent worlds that fail new tests from blocking said tests. Of course the downside of this is it means broken worlds are not fixed immediately, but it prevents the tests from sitting in PR hell and allows new worlds to have the tests to prevent them from making the same error. Should this be included here? I imagine a field like unsafe_opt_out_tests: list[str] which just names the tests to be opted out. Playing along with the decorator suggestion, this is also something that could be implemented there by using the wrapped function's name as a filter when iterating the testableworlds

[BadMagic100]

Given how often this pattern is used, it seems like it would be desirable to make it easier to reuse this as it's getting very bulky and inconvenient to use. It'll also make future refactoring easier.

One option that comes to mind but may be neither optimal nor trivial is to make an decorator that automatically implements this pattern, allowing each test to be implemented as a function of world/options. The iteration and subtests would then be handled opaquely under the hood for a majority of tests

[BadMagic100]

Of course I have more thoughts just after reviewing

• I'd like to see the docs updated indicating how worlds can edit their presets
• To foresee a common complaint, adding additional presets will increase the running time so I think we need to communicate clearly that these should be used thoughtfully and sparingly rather than, say, applying every possible combination of settings

[Mysteryem]

I don't know about this. What this PR wants seems exactly like what WorldTestBase already is or was trying to be. WorldTestBase does not currently satisfy what this PR wants because most core tests have not been added to the tests run by WorldTestBase.

Why not move more of the core tests to WorldTestBase?

Merging this PR would end up with two world testing setups that are almost the same, which I think is redundant and would be confusing.

[NewSoupVi]

Why not move more of the core tests to WorldTestBase?

We could, but then we really really really need to encourage the use of run_default_tests = False, or make it default to False, or make another layer of subclassing where WorldTestBase only has the setUp and helpers, and then we have a "WorldTestBaseWithDefaultTests" that has the default tests.

Right now, every single TestCase subclassing WorldTestBase running Fill is seen as a problem by most of the important™️members of the community, and this is completely intransparent to world devs such that they do it "by accident". In this scenario, I would insist that we find a disambiguation between WorldTestBase as the "convenient helper class for custom unit tests" and the "registering non-default options to run default tests on" class

[NewSoupVi]

Also, we would need a mechanism where core can run these WorldTestBase tests on default options, because that part needs to be retained for sure, so I guess that would be some sort of dynamic class creation

[NewSoupVi]

And finally, this would mean that WorldTestBase would be like, really big, instead of now where the unit tests are neatly split across multiple files in multiple folders. Unless there is a good way to split one class across multiple files?
Idk, I'm a bit skeptical, unless we find solutions to all these things & someone is willing to put in the effort of making all of it.

If someone wants to start work on that, doing the "disambiguating" of WorldTestBase would be the first step. I would see that as a generically good change anyway (Highlighting this because I don't want to be seen as just unproductively shooting down this idea)
Dynamically creating one subclass per game with default options could be the second step, which would then allow us to actually move these tests over.

[NewSoupVi]

Oh, also also also, one thing I like about this solution is that we have an easy mechanism for "rewarding" worlds who register more presets than just the default one, e.g. by just running len(world_class.presets). I guess we could have some sort of metaclass for WorldTestBase that counts subclasses by WorldTestBase.game instead? A metaclass like this could also prevent running the default tests on the same options twice, which I think is important.

[NewSoupVi]

I keep thinking of more things lol

We have some sort of multiprocessing setup for our unit tests right? Idk if that is gonna work if all tests are in one class, which may prevent us from making a fast "run all default unit tests on an apworld on install" which we want to do in the future.

pytest-xdist multiprocessing does appear to work with multiple functions on the same unittest.TestCase subclass, so this is not a concern.

[Mysteryem]

Why not move more of the core tests to WorldTestBase?

We could, but then we really really really need to encourage the use of run_default_tests = False, or make it default to False, or make another layer of subclassing where WorldTestBase only has the setUp and helpers, and then we have a "WorldTestBaseWithDefaultTests" that has the default tests.

The current WorldTestBase does have its problems, and I think looking into fixing those problems and what a fixed WorldTestBase could be like would be useful to compare the benefits and problems of a fixed WorldTestBase to the changes in this PR.

And finally, this would mean that WorldTestBase would be like, really big, instead of now where the unit tests are neatly split across multiple files in multiple folders. Idk, I'm not a big fan, unless we find solutions to all these things

There is no requirement to move the test implementations into WorldTestBase, the implementations could remain organised in separate files and folders and either be called directly or mixed into WorldTestBase (or whatever test class core ends up with). I do have concerns that mixing in test methods could make it more difficult to run individual tests from an IDE, however.

Oh, also also also, one thing I like about this solution is that we have an easy mechanism for "rewarding" worlds who register more presets than just the default one, e.g. by just running len(world_class.presets). I guess we could have some sort of metaclass for WorldTestBase that counts subclasses by WorldTestBase.game instead?

__init_subclass__ also exists and can be used in many cases instead of needing a metaclass.

We have some sort of multiprocessing setup for our unit tests right? Idk if that is gonna work if all tests are in one class, which may prevent us from making a fast "run all default unit tests on an apworld on install" which we want to do in the future.

I don't think there will be any issues with pytest-xdist, but separating out only 'default unit tests' might be more complicated, I am not sure.

[NewSoupVi]

Black Sliver and I discussed all of these points in Discord:

https://discord.com/channels/731205301247803413/731214280439103580/1424353137598402630

I have come to the conclusion that "I don't hate it" (which is high praise in German culture)

We came to a lot of the same conclusions as BadMagic

[NewSoupVi]

Here's the final result of that discussion from my side https://discord.com/channels/731205301247803413/731214280439103580/1424359725151293450

I think having this is reasonable maybe:

WorldClassTestBase, which just takes a "game" ClassVar & maybe validates that the game exists / that it can grab the World class from the game & does nothing else really
A bunch of WorldClassTestBaseMixins.XYZ, each of which contain one of the default tests that just check class-level stuff
ExtensiveWorldClassTestBase, which subclasses all the mixins and runs all class-level tests on one world

WorldTestBase, which subclasses WorldClassTestBase, has the helpers and setUp and options classvar from current WorldTestBase but not the default tests
A bunch of WorldTestBaseMixins.XYZ mixins, each of which contains some set of the default tests that need a multiworld to be created
ExtensiveWorldTestBase, which subclasses all the mixins and runs all tests on one world that need a multiworld to be created

And then we dynamically create one ExtensiveWorldClassTestBase and one ExtensiveWorldTestBase for all worlds + allow worlds to create additional ExtensiveWorldTestBase subclasses for non-default options

And then finally we have the custom runner that filters by game to be able to run just one game