Disable a test that broke the current build #655
Conversation
| @@ -1,6 +1,8 @@ | |||
| #include "gaia/logger.hpp" | |||
| #include "gaia/events.hpp" | |||
|
|
|||
| // #define TEST_FAILURES | |||
There was a problem hiding this comment.
I'm a bit confused about this approach of disabling the tests. Why have this commented line here? It's not as if you'd ever want to uncomment it, right?
And then the #ifdef TEST_FAILURES doesn't sound intuitive - the code is enabled if there are test failures?
I think it would be more descriptive to call the macro variable something like ENABLE_FAILING_TESTS and then enclose the ruleset in:
#ifndef ENABLE_FAILING_TESTS
...
#endif
I wouldn't keep any commented out define line either.
There was a problem hiding this comment.
With Gregory enhancing gaiat while I am writing gaiat tests, I try to get ahead of him with tests that won't pass yet. For bettor or worse, I have a convention in the test files to disable tests that don't pass yet. The disabling of this one test simply follows the pattern that is already in place. It's a reminder to me of tests to re-test when there is a new commit from Gregory. Eventually, all of the #ifdef statements will be gone. BTW the name TEST_FAILURES means "test the failures" and is facilitated by uncommenting the #define. This is transitional as Gregory and I move forward with gaiat.
There was a problem hiding this comment.
The ambiguity is that in this context "test" is used more like a noun than a verb.
Also, do you really want to test all the failures in the file, or do you want to test them one by one (in which case the top define is irrelevant)?
There was a problem hiding this comment.
Is it possible to make this test part of Gregory's branch? You can update the PR to merge the change to his branch. Logistic wise, we won't need to re-enable this after Gregory's change is in and the test can be used to test gaiat change in the local branch before merging to master.
There was a problem hiding this comment.
I'm going to work with Gregory to find better ways to synchronize the language enhancements with the tests. Obviously this shouldn't interfere with the build. Thanks for the suggestions.
This was a synchronization error between the Language tests and Language implementation. The test will work when the latest Language updates are merged - but not yet.
This removes one test that was causing a crash in the build. It will be added back later.