Unit test module

[theogiraudet]

Context

Bookshelf becomes bigger and bigger and it's become harder to confirm the different modules work as expected with the new Minecraft's updates.
To overcome this, @Leirof started to develop some unit tests but these one are not really maintain or extend over the time by the different developers.

Proposition

I propose to normalize the way to do unit testing thanks to a new Bookshelf module in the same way we do with the bs.log module. I also propose to officially add unit testing to the documentation as a mandatory step when contributing a new feature.

How?

To develop this module, we can take inspiration from the existing unit testing frameworks like Junit in the Java world.
In this module, I propose several assert features enabling the verification of some information.

Module features

/!\ In my proposition, this module will massively use macro, I consider this not a big problem since this kind of module isn't used in a production environment /!\

Each assert feature takes in parameters the tested feature as a string.

bs.test:init

Initialize a new testing session by resetting the failed tests and the succeed tests.

bs.test:assert_score_true

This feature takes an entity selector and an objective and checks if the score of the given entity(-ies) matches any values other than 0.

bs.test:assert_score_false

This feature takes an entity selector and an objective and checks if the score of the given entity(-ies) matches 0.

bs.test:assert_score_equals

This feature takes two entity selectors and two objectives and checks if the two scores match. Fail if the selectors don't match a unique entity (each).

bs.test:assert_score_not_equals

This feature takes two entity selectors and two objectives and checks if the two scores don't match. Fail if the selectors don't match a unique entity (each).

bs.test:assert_score_matches

This feature takes an entity selector, an objectif and an integer and checks if the score matches the integer. Fail if the selectors don't match a unique entity (each).

bs.test:assert_score_not_matches

This feature takes an entity selector, an objectif and an integer and checks if the score doesn't match the integer. Fail if the selectors don't match a unique entity (each).

bs.test:assert_score_defined

This feature takes an entity selector and an objective and checks if the selected entities have a score on this objective.

bs.test:assert_score_undefined

This feature takes an entity selector and an objective and checks if the selected entities don't have a score on this objective.

bs.test:assert_nbt_equals

This feature takes two NBT holder categories (block, entity, storage), two NBT holders matching the respecting categories and two NBT paths and checks if the two NBT elements match. Fail if the selector of an entity don't match a unique entity.

bs.test:assert_nbt_not_equals

This feature takes two NBT holder categories (block, entity, storage), two NBT holders matching the respecting categories and two NBT paths and checks if the two NBT elements don't match. Fail if the selector of an entity don't match a unique entity.

bs.test:assert_nbt_defined

This feature takes an NBT holder category (block, entity, storage), a NBT holder matching the category and a NBT path and checks if the NBT element is defined. Fail if the selector of an entity don't match a unique entity.

bs.test:assert_nbt_undefined

This feature takes an NBT holder category (block, entity, storage), a NBT holder matching the category and a NBT path and checks if the NBT element is undefined. Fail if the selector of an entity don't match a unique entity.

bs.test:assert_nbt_array_contains

This feature takes an NBT holder category (block, entity, storage), a NBT holder matching the category, a NBT path to a NBT array and a NBT element and checks if the NBT array contains the element. Fail if the selector of an entity don't match a unique entity or if the NBT element indicated by the pass isn't an array.

bs.test:assert_nbt_array_not_contains

This feature takes an NBT holder category (block, entity, storage), a NBT holder matching the category, a NBT path to a NBT array and a NBT element and checks if the NBT array doesn't contain the element. Fail if the selector of an entity don't match a unique entity or if the NBT element indicated by the pass isn't an array.

bs.test:assert_nbt_compound_contains

This feature takes an NBT holder category (block, entity, storage), a NBT holder matching the category, a NBT path to a NBT compound and a string and checks if the NBT compound contains the string as key. Fail if the selector of an entity don't match a unique entity or if the NBT element indicated by the pass isn't an array.

bs.test:assert_nbt_compound_not_contains

This feature takes an NBT holder category (block, entity, storage), a NBT holder matching the category, a NBT path to a NBT compound and a string and checks if the NBT compound doesn't contain the string as key. Fail if the selector of an entity don't match a unique entity or if the NBT element indicated by the pass isn't an array.

bs.test:assert_entity_contains_tags

This feature takes an entity selector and an array of tags and checks if the selected entities have all the tags of the array.

bs.test:assert_entity_not_contains_tags

This feature takes an entity selector and an array of tags and checks if the selected entities have none of the tags in the array.

bs.test:assert_function_succeed

This feature takes a function path (with macro parameters or not) and checks if the value returned by the function isn't 0.

bs.test:assert_function_failed

This feature takes a function path (with macro parameters or not) and checks if the value returned by the function is 0.

bs.test:manual_assert

Sometime, a feature cannot be automatically tested, for instance if a message is displayed in the player chat or not (log features).
This feature takes a message describing the expected result and will display the message to the user with two buttons: an "accept" button and a "reject" button. The player clicks on one or the other according to if they observe the behaviour described by the message or not.

bs.test:end

Displays the number of succeed tests, failed tests and for each failed tests, the reason why the test failed.

bs.test:run

Top-level function, takes a context between all, namespace, module, feature, test and an identifier and execute the tests for the chosen context. before that, calls bs.test:init. After that, calls bs.test:end. Before all tests of a feature, calls the __before_all__ function of the feature (see next part). Before each tests of a feature, calls the __before_each__ function of the feature (see next part).

bs.test:load_test_namespace

Takes a datapack namespace and load the list of all modules, features and tests of the namespace (see next part).

Datapack for unit testing

Currently, unit testing are in a separated data packs. I think this approach is the good one.
Here is the organization I propose for testing data packs:

• Each tested module has its own folder.
• Each tested feature has its own folder into its module folder.
• A tested feature can have one or several testing function.
• A function can have one or several assertion.
• Each feature has its own __before_all__ function initializing the execution context (summoning an entity, setting scores, giving tags...)
• Each feature can have its own __before_each__ function, resetting, if needed, the context between two function calls of a same feature
• Each feature folder has its own __tests__.mcfunction with one data modify storage bs:test <tested namespace>. features.<feature name> set value [<tests>], with <tests>, the list of all the tests functions of the feature
• Each module folder has a __features__.mcfunction with one data modify storage bs:test <tested namespace>.modules.<module name> set value [<features>], with <features>, the list of all the features of the module
• At /data/<namespace>/functions/ a __modules__.mcfunction with one data modify storage bs:test <tested namespace>.module_list set value [<modules>], with <modules>, the list of all the modules
• At /data/<namespace>/functions/ a __load__.mcfunction with a call to bs.test:load_test_namespace with the current namespace as parameters

Examples

I want to test the bs.math:sqrtfunction:

bs.dp_test:math/sqrt/__before_all__

function bs.math:__load__

bs.dp_test:math/sqrt/__before_each__

scoreboard players remove #sqrt bs.out

bs.dp_test:math/sqrt/test_correct_values

scoreboard players set #sqrt bs.in 16
function #bs.math:sqrt
function #bs.test:assert_score_matches {feature: "bs.math:sqrt", entity: "#sqrt", objective: "bs.out", value: 4}

bs.dp_test:math/sqrt/__tests__

data modify storage bs.test bs.dp_test.features.math.sqrt set value [ "math/sqrt/test_correct_values"]

bs.dp_test:math/__features__

data modify storage bs.test bs.dp_test.features.math.sqrt set value ["math/sqrt/test_correct_values"]

bs.dp_test:math/__features__

data modify storage bs.test bs.dp_test.modules.math set value ["math/sqrt"]

bs.dp_test:math/__modules__

data modify storage bs.test bs.dp_test.module_list set value ["math"]

bs.dp_test:__load__

function bs.test:load_test_namespace {namespace: "bs.dp_test"}

To run all the tests of the namespace:

function bs.test:run {level: "namespace", value: "bs.dp_test"}

Result, something like that, display to the user:

1/1 tests succeed.

[aksiome]

I'm not a fan of having multiple files for each feature. The point of having a test module is to write test faster and having to create a lot of single file for each test of a feature is imo a bad idea. I think we should have a single file that perform all tests on a feature. This way we only have a list of feature to test and not a big list. Sure it's less precise for the report because we only know which feature failed and not which test exactly but since we can get that information from the assert, i think it's not an issue.

This would look like this:

I want to test the bs.math:sqrt function:

bs.dp_test:math/__setup__

bs.dp_test:math/__teardown__

bs.dp_test:math/sqrt

# FEATURE SETUP

# TESTS
scoreboard players set #sqrt bs.in 16
function #bs.math:sqrt
function #bs.test:assert_score_matches {feature: "bs.math:sqrt", entity: "#sqrt", objective: "bs.out", value: 4}

# ... other tests

# FEATURE TEARDOWN

The rest is the same as what you proposed with feature list and module list.

Also if we abstract it more it's like having a class (module) that control multiple tests (features). With what you proposed there is an other layer of complexity on top of that.

[theogiraudet]

One test isn't one assert, for me there is one test per case to manage but a test can has several asserts.

[aksiome]

Sure i know that but i prefer to have multiple tests inside the same file.

[theogiraudet]

The developper choose how they want to organize their tests but I think we need to manage the case where there is several tests/functions for one feature.

[aksiome]

Yeah i agree and if we need that we just create a bs.dp_test:othertestgroup. We are too focused on modules and features imo and other datapack may not necessarly use this structure. If i want to control more the test we can still have this instead bs.dp_test:sqrt with multiple tests inside

[aksiome]

In reality what i'm proposing is almost the same, we just get rid of the __features__ and __modules__ to only have a __tests__. Then we can organize our tests as we like for sqrt for example we only need a single test but maybe for some other feature we'll need multiple tests.

[theogiraudet]

Yeah, there is a ambiguity between feature as we use in Bookshelf.
__test__ isn't enough because we don't necessary want to execute all the tests (or only one). It's common in testing to have one or several class files to test only one aspect of the program, so a feature of the program.

[aksiome]

Should we keep this since we're now relying on packtest for unit tests?