Hey Jose,
On 18/02/15 17:10, Jose Fonseca wrote:
FYI, you'll need to rebase on top of
http://cgit.freedesktop.org/piglit/commit/?id=83bc6862386b2d465879bcd372d61ec754534970
, so that we use the standard C99 syntax for variadic macros. You
might need to manually update the moved code, as git might just mark
it as a conflict.
Sorry for breaking the windows build... No probs, I'll rebase it.
Concerning the actual change, just a general remark: these macros
save typing but they make the test code harder to understand
I think there are other approaches that achieve the same amount of
type-saving, without compromising readbility.
For example, we could have a
// global contaning the current substest result
// TODO: Make it a enum piglit_result
bool current_subtest_result;
char current_subtest_message[4096];
piglit_subtest_begin() {
current_subtest_result = true;
current_subtest_message[0] = 0;
}
void
piglit_subtest_check(bool condition, format, ...) {
bool pass = piglit_check_gl_error((error));
if (!pass) {
// FIXME: append format+va_args to current_subtest_message.
}
current_subtest_result = current_subtest_result && pass;
}
void
piglit_subtest_check_gl_error(error) {
...
}
piglit_subtest_end()
piglit_report_subtest_result(current_subtest_result ? PIGLIT_PASS :
PIGLIT_FAIL, current_subtest_message);
}
And the test would do
piglit_subtest_begin();
piglit_subtest_check_gl_error(error, etc);
piglit_subtest_check_gl_error(error, etc);
piglit_subtest_check(cond, etc);
piglit_subtest_end();
Not sure this form saves anything since one sub-test would be one line
in my case instead of 3 in this case.
Have a look at
http://cgit.freedesktop.org/~mperes/piglit/commit/?h=dsa&id=c08558659f7967410ab68f39c3d768e41e35bc6d
What you proposed is a good idea for the whole test though.
piglit_test_begin() would set current_test_result
to keep track of the state and the other functions would follow the
state. I'm not a fan of the global variable
too as what will happen if we have interleaved tests?
In any case, I'm sure some tests will not work with this model. So,
instead of sharing those function and making them
very confusing, I would argue that outside of trivial cases (like the
macros I proposed), you should re-write your own
way of checking. As for the "confusing" argument, not sure how much
simpler can this code be (*no side effect*). It is
also less prone to copy/paste errors.
In my opinion, we won't reach an agreement that would work in every case
or, if we do, people will not use the code
added because it will be too complex. Instead, let's just let devs
handle their test code as it pleases them. If there are
some trivial cases common-enough, then we can add them to
piglit-util.sh. I guess the question now is, are those
common-enough outside of DSA?
Martin
_______________________________________________
Piglit mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/piglit
