Create more general Test Framework and Organize the bulk of current tests #67
Conversation
… current tests into new framework
| // This is used by the test runner code to know what the expected value is | ||
| // For a simple example like this, a single eval is all that's required, but for | ||
| // more complex tests, this may not be the case. | ||
| std::string eval() { |
There was a problem hiding this comment.
This is great and will probably be super useful for more complicated randomly generated expressions.
| testing::ValuesIn(genIntegerTests(4, | ||
| true, | ||
| true, | ||
| {IntegerParam(-2147483648), |
There was a problem hiding this comment.
At some point we should add in more edge case integers. See 'IR_LoadConstant64::do_codegen` which picks one of 5 different instructions for doing the load depending on the size/sign of the value.
| } | ||
|
|
||
| // In the interest of speed, we want to share the same thread/compiler across | ||
| // all the tests in this suite, so we have to over-ride this. |
There was a problem hiding this comment.
👍 without this the tests can take a while
| void TearDown() {} | ||
|
|
||
| // Common Resources Across all Tests in the Suite | ||
| static std::thread runtime_thread; |
There was a problem hiding this comment.
This seems kinda weird, but maybe this is what you're supposed to do for gtests?
There was a problem hiding this comment.
Well, using gtest's Fixture support was the best way I could think of to share the thread/compiler/runner across all tests.
However I don't really like it for several reasons:
- Since the Fixture has to extend with the parameter, you can't easily have 2 different types of parameters to generate tests, unless you make a new fixture, which means a new thread,etc.
- Lots of boilerplate
So this is an area for improvement. I think it would be nicer to have a "global" runner with/without the kernel, and have some sort of mechanism to "refresh/wipe" the state so we can ensure tests are hermetic. I ran into some difficulty trying to do the former though.
There might also be a better way to do the parameterized tests - https://github.com/google/googletest/blob/master/googletest/docs/advanced.md#creating-value-parameterized-abstract-tests so we can have a common library of some sorts.
There was a problem hiding this comment.
I looked at this a little bit more and I think what you have now is good. The static stuff in the fixture class seemed weird to me at first because I didn't realize each test would get its own fixture.
I agree we need a better way to reset the state of the compiler/runtime between tests. I don't think it would be too hard to add a reset_state to the compiler that clears out all the variable/constant/function/type definitions. Doing a fast reset of the runtime is going to be much harder though. There's some hacky stuff we could do like saving the entire runtime GOAL memory before a test and restoring it, but this won't catch everything.
| GoalTest::CompilerTestRunner WithGameTests::runner; | ||
|
|
||
| // TODO - havn't done anything with these really yet | ||
| TEST_F(WithGameTests, All) { |
There was a problem hiding this comment.
This is probably for another PR, but I think a lot of these tests just call a single function with a few arguments and check the result. I imagine we could have a Call Function test that generates these without needing all these separate test .gc files.
xTVaser
force-pushed
the
generative-tests
branch
4 times, most recently
from
068d2b7
to
7540e6a
Compare
xTVaser
force-pushed
the
generative-tests
branch
from
7540e6a
to
c34c0a8
Compare
Pull Request Test Coverage Report for Build 298482569
💛 - Coveralls |
xTVaser
force-pushed
the
generative-tests
branch
from
5f86b83
to
4429632
Compare
| test_deftype.cpp | ||
| test_pretty_print.cpp |
There was a problem hiding this comment.
Was this removed by accident?
| std::string& testCategory, | ||
| const std::string& test_file, | ||
| const std::vector<std::string>& expected, | ||
| MatchParam<int> truncate) { |
There was a problem hiding this comment.
Don't worry about changing this now, but C++17 now has std::optional which is better than the MatchParam thing. Eventually I'll switch everything to std::optional
| @@ -5,7 +5,7 @@ jobs: | |||
| common: | |||
| # TODO - Start using clang-tidy - requires a bit more setup though - http://clang.llvm.org/docs/HowToSetupToolingForLLVM.html | |||
| name: Linting & Formatting | |||
| runs-on: ubuntu-latest | |||
| runs-on: ubuntu-20.04 | |||
This PR doesn't yet add new torture tests, I figure that can come later. Just trying to replicate the current tests with the new framework and work-out the kinks
Of which there are still a few.