Skip to content

Conversation

jlindgren90
Copy link
Contributor

Converting RUN_TEST() from a macro to a function significantly reduces the size of the compiled binary. On amd64, the largest test runner in the test suite (testsample_DefaultsThroughCommandLine_runner.o) was reduced from 3.4 kB to 2.4 kB (stripped).

Depends on #453.

Copy link
Member

@mvandervoord mvandervoord left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall this is looking really good. It appears you're dropping parameterized support, though? Why wouldn't we just replace create_runtest's output with a function instead of a macro, but otherwise keep it functionally the same?

@jlindgren90
Copy link
Contributor Author

Parameterized tests are still alive and well, I'm just handling it a little different. Each parameter set gets wrapped in a tiny void(void) function that then gets handled the same as any other test. The end result is the same functionality but in a smaller binary.

@jlindgren90
Copy link
Contributor Author

e.g. testparameterized_Runner.c contains:

static void runner_args1_test_TheseShouldAllPass(void)
{
    test_TheseShouldAllPass(0);
}
static void runner_args2_test_TheseShouldAllPass(void)
{
    test_TheseShouldAllPass(44);
}
static void runner_args3_test_TheseShouldAllPass(void)
{
    test_TheseShouldAllPass((90)+9);
}

And then:

  run_test(runner_args1_test_TheseShouldAllPass, "test_TheseShouldAllPass(0)", 78);
  run_test(runner_args2_test_TheseShouldAllPass, "test_TheseShouldAllPass(44)", 78);
  run_test(runner_args3_test_TheseShouldAllPass, "test_TheseShouldAllPass((90)+9)", 78);

@mvandervoord
Copy link
Member

I'm understanding now. I just hadn't dug far enough.
:)

Awesome. I'm definitely all for space savings!

@mvandervoord
Copy link
Member

Cool. I'm feeling good about these changes. Thanks for all the work @jlindgren90

@Letme -- I'm pausing here to give you a chance to comment before I merge. I know you're another person who has a lot of valuable thoughts in this area.

@mvandervoord mvandervoord merged commit addd60e into ThrowTheSwitch:master Oct 24, 2019
@jlindgren90 jlindgren90 deleted the common-run-test branch October 29, 2019 17:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants