Update global tests (part 1)

[RadWolfie]

Instead of contributors manually input strings of hex index / name of api. I simply replace all into parameters to remove any possible typo mistakes. This way, the compiler/runtime will do the work for us easier.

Instead of manually updating each one which will take hours. I used a couple of regexes to replace all of it in one go. It only took me about 15 seconds and 15 minutes to replace what I had missed. I also spent 45 minutes verifying that each api test bodies were correctly replaced.

In this change:

• void test_<name of api>(void) been replaced to void test_<name of api>(int func_num, const char* func_name)
  • With this replacement, local variables of func_num and func_name of all tests had been removed.
• renamed kernel_thunk_table to kernel_api_tests per @PatrickvL suggestion request
• renamed func_table.h to func_tests.h

I also confirmed all implemented tests did output the correct hex index value in the log file.

[PatrickvL]

Nice work, so consider the following to be less important than what's already been done here:

If there's a chance the test function signature have to change again, then perhaps it's handy to define a macro for it? That way, if the arguments passed into each test need to change, only the macro would have to be updated. And if that's combined with macros for the printing functions as well (the ones that currently use the test arguments) then also the printing can be redeclared centrally, without having to alter all tests.

Of course, careful consideration is needed whether macros are acceptable for such an application...

[PatrickvL]

#define TEST_FUNC(name) void name(int api_num, char *api_name)

TEST_FUNC(RtlLower)
{
  // ...
  TEST_FINISHED();
}

[RadWolfie]

Hmm, you're missing the starting of the test function aka print_test_header(func_num, func_name);.

So it would be something like...

#define TEST_FUNC(name) void name(int api_num, char* api_name)
#define TEST_START() print_test_header(api_num, api_name); \
    BOOL test_passed = 1
#define TEST_FINISHED() print_test_footer(api_num, api_name, test_passed)

TEST_FUNC(RtlLower)
{
    TEST_START();
    // ...
    TEST_FINISHED();
}

However, there are some other tests that are using tests_passed instead of test_passed name. Meaning those will need to replace as well.

[RadWolfie]

Then the func_tests.h will be better to rename as api_tests.h, correct? In order to reflect the changes from func_<num|name> to api_<num|name>.

[PatrickvL]

Or you could pass that in as an argument to that macro?

[RadWolfie]

Or you could pass that in as an argument to that macro?

Good question, should we? Since we have GEN_ prefix defines that are locked to local test_passed variable which aren't passed as argument in the macros. If you do feel TEST_FINISHED should pass test boolean as argument, then I will accept it anyway.

[PatrickvL]

Or you could pass that in as an argument to that macro?

Good question, should we? Since we have GEN_ prefix defines that are locked to local test_passed variable which aren't passed as argument in the macros. If you do feel TEST_FINISHED should pass test boolean as argument, then I will accept it anyway.

The test_passed variable is declared once via the define ASSERT_HEADER and a few times independently (but those could all use the ASSERT_HEADER). [Nearly all test_passed reads/writes are similar, and could be done via macro's as well, but it feels a unnecessary to start using macro's for otherwise simple declarations.]

Like test_passed, the func_num and func_name are also sometimes used (mostly by a print_test_footer call), but those could also all be moved inside a define (if that's the way to go).

So, perhaps the print_test_header and print_test_footer calls could be wrapped in a macro, so that the func_num and func_name variables no longer appear in the actual test code (only in the macro's, including the proposed TEST_FUNC macro.)

As for when to pass in an argument to a macro, and when to just use it in the macro (assuming the symbol/argument/variable will exist), I think that for func_num and func_name they should not even appear in the code anymore (so only be mentioned in macro declarations) - likely best done by moving the print_test_header and print_test_footer into macro's. With that, the amount of mentions of test_passed is reduced significantly, and quite a few of the ones that remain are inside ASSERT macro's already. So don't keeping around a few mentions of test_passed in test code seems not worth the trouble to move it into a macro.

Besides, assert_NTSTATUS always appears like this : tests_passed &= assert_NTSTATUS(result, ... , func_name); and could thus incorporate the tests_passed &= part, which would remove many of the remaining tests_passed occurences too.

[RadWolfie]

Actually, we could move test_passed &= into pre-existing macros. As for the manual check such as:

if(ret == 1) {
    test_passed &= 1;
}
else {
    test_passed &= 0;
}

can be convert to use GEN_CHECK macro.

For any custom checks in the tests that aren't using pre-existing macros. We could make additional macro specifically for this purpose. Which could be TEST_FAILED() macro as test_passed = 0. Then test_passed become completely hidden from all of api test's body. I can also add a comment to tell creator of the test not to use the variable for their own purpose.

Though, for ^ purpose it can possibility take a while to find and replace them. Which will make it harder to review in this pull request. I would like to handle this test(s)_passed replacement in a separate pull request if you don't mind.

[RadWolfie]

Finalize discussion changes has been pushed recently. Of course excluding minor changes for part 2 pull request mainly to have better readability review after this majority changes.