dangmoody / temper Goto Github PK

Clang 13 has a bug where anything that gets #defined and starts with a double-underscore will trigger compiler errors EVEN IF you have -Wreserved-identifier warning disabled. This is on Windows only it looks like.

This is possibly broken for single underscore defines too since those are also reserved by the compiler IIRC, but I've not tested that.

There's a few GitHub issues for it on the Clang GitHub. It looks like someone has already patched it though, but that fix doesn't look to be out yet.

We can avoid this whole mess by just not having any #define start with a double underscore. So I propose renaming the following #defines to:

• TEMPERDEV__TEST_INFO_FETCHER -> TEMPERDEV__DEFINE_TEST_INFO_FETCHER.
• __temper_test_info_fetcher_ -> TEMPERDEV__TEST_INFO_FETCHER_.

I have an experimental version of Temper with these changes already running in HLML if you want to preview it.

The only concern I have about this is that those names are quite similar and might affect readability, but those are the best names I can come up with right now.

Not sure how I want to go about implementing this. Doing a bunch of if checks inside the build.bat to check for whether the file is .c or .cpp and call either clang or clang++ respectively seems ugly.

I imagine I want a separate build script.

Because of the fact that we're having to expose test functions via __declspec( dllexport ) a static library gets created automatically by the compiler. I'm not sure this is something we want. We load the game .exe into itself manually, so there's no need for the static library.

There might be a compiler flag to disable this behaviour, but I haven't checked.

This isn't priority because we still need to flesh out the rest of the library, but it's something that we'll probably want to fix come release.

We believe it may be it may be possible to have floating point equality checking functions that DO NOT require the user to provide their own epsilon values. Investigate and experiment with the subjects spoken about here:

https://realtimecollisiondetection.net/blog/?p=89
https://www.gamedevs.org/uploads/numerical-robustness-geometric-calculations.pdf

Any further thoughts and readings into this subject matter will be posted below.

So this morning I added Aborts to the conditional checks as it's another thing I use ALOT and felt tantrum just wouldn't be complete without. After submitting it I realised what I'd writted just wasn't going to hold water especially if we start adding fixtures or anyone starts writing any helper functions for their tests.

Currently all the abort sequence does is call return. So if your test calls a function with an abort in it, the test doesn't stop.

The solution: Each test needs to run it's own thread. When an abort is triggered, the thread is terminated.

I found a simple solution I was going to implement but suddenly realised it was for linux and mac only - it was using the pthread library. I next went to MSDN and it turned into a very deep rabbit hole that requires discussion. We'll need to discuss this over a call further:

Threads for C
https://docs.microsoft.com/en-us/cpp/parallel/sample-multithread-c-program?view=msvc-160

Creating and terminating in C++
https://docs.microsoft.com/en-us/windows/win32/api/processthreadsapi/nf-processthreadsapi-createthread
https://docs.microsoft.com/en-us/windows/win32/procthread/terminating-a-thread

The macro prints out the logic, if you want the values we could just make it do

TEMPER_CHECK_EQUAL("We expected player health to be 5, instead it's: %f", player.health);

Seriously, this tilted me when I just wanted to know what was going on.

As found by @Mike430, if you want to compare doubles you have to pass it into TEMPER_CHECK_FLOAT_EQUALS, which is not ideal for obvious reasons. Oops!

So we need CHECK functions for doubles too, to reduce friction and make life easier.

The only problem with that is we have TEMPER_CHECK_ALMOST_EQUALS, which is ambiguous. So we need to find a new name for that so we can differentiate between float and double.

If anyone has any ideas I'm all ears!

Probably want separate build scripts for these (especially considering you can't compile with MSVC on Linux!).

When tests fail, I am forced to debug the code to see the actual vs expected results. Ideally, this should automatically print out to the log.

Add a Quit or Exit option to the API to have Temper shutdown if a given test fails.

I've proven that testing without this is more robust and is the preferred way of doing things. Providing alternative code paths is messy for the header which IS THE CODE WE SHIP TO FOLKS. We can drop all the alternative macros, alternative counters, etc.

Something I think would benefit those working within the terminal is an option to only print anything at all if and only if a check fails. At that point, print the information about the test. If we are to do this it would also be a good idea to look at doing text output suppression so you don't get any of the noise of the program running. Just a concise list of failing tests & accompanying errors.

This will be updated regularly.

This will be closed when all of these are ticked off.

Build configs:

• win64-msvc (C)
• win64-msvc (C++)
• win64-clang
• win64-clang++
• win64-gcc
• win64-g++
• linux-clang
• linux-clang++
• linux-gcc
• linux-g++
• macos-clang
• macos-clang++
• macos-gcc
• macos-g++

Make the test listing consistent across compilers.

Separately, for multi-unit compilations, add alphabetical ordering across all tests.

Make alphabetical and code defined ordering an option on startup.

Recommend Alphabetical as default, personally. Not biased at all. Promise!

There are perfectly valid reasons for wanting to do this, and it should definitely be done.

TL;DR: From a usability standpoint function pointers that are overridable are just better and come with significantly less friction for a performance cost that simply will not matter in the grand scheme of things.

This system should be replaced with a struct full of function pointers.

Currently in Temper the way you override a function is done like so (example):

#define TEMPER_IMPLEMENTATION
#define TEMPERDEV__ON_BEFORE_TEST OnBeforeTest
#define TEMPERDEV__ON_AFTER_TEST OnAfterTest
// and so on...
#include <temper/temper.h>

There are a few usability problems with this approach.

• In the above example, I would also need forward declarations of each function before the above code. This is a somewhat arbitrary requirement vs. "Just set the pointer of those functions to the ones you have defined".
• Setting these overrides must be done before including the main Temper header. By using function pointers they would have to be overridden in the entry point where the rest of the Temper code but it would ultimately provide greater flexibility for people who would be doing things beyond your most basic Temper entry point implementation.
• The defines on their own give absolutely no indication of what the function returns or what it's parameters are. Every time you want to override one of these functions you end up having to look at the #define you want and then go find the corresponding default function, take the return type and parameters from that, and then fill out your own function somewhere. Function pointers would give you all of this information up front; removing friction.

When I initially implemented this system I wanted to mainly avoid having a struct full of function pointers and storing a pointer to that struct in the main temper test context. I thought having a bunch of #defines would be just as simple to work with and I'd avoid having a bunch of function pointers taking 8 bytes each stored in a struct, ultimately leading to cache misses that I thought were otherwise avoidable.

However, the total cost of adding all of these function pointers will be far outweighed by the sheer usability improvements that come from it. Temper will still run fast with this change in.

Currently when defining a test outside the test header, it causes a compiler warning as the test is neither static nor forward declared. The issue is that we just instantly declare & define our invoker function which will package up all the information about our test function when the compiler wishes for it to either be static or for it to be forward declared. However;

1. We cannot make it static because then it's not exported/exposed to our module handle query

2. We cannot forward declare it (at the moment) because we still rely on COUNTER which would be incremented by the forward declaration.

A hacky solution that will get us to release but won't be elegant would be to just have a START_TEST_REGION (used before all tests in a file) and END_TEST_REGION (used after) macro that would just disable and enable compiler warning suppression. This would also stop the end user getting useful warnings about THEIR code though which is bad.

Was reading the README again and saw that we had listed as WIP but there was no issue made for it, so I'm making one now before I forget (again).