Maciej W. Rozycki wrote:
On Wed, 3 Jan 2024, Hans-Peter Nilsson wrote:
The test execution timeout is different from the tool execution timeout
where it is GCC execution that is being guarded against taking excessive
amount of time on the test host rather than the resulting test case
executable run on the target afterwards, as concerned here. GCC already
has a `dg-timeout-factor' setting for the tool execution timeout, but has
no means to increase the test execution timeout. The GCC side of these
changes adds a corresponding `dg-test-timeout-factor' setting.
Hmm. I think it would be more correct to emphasize that the
existing dg-timeout-factor affects both the tool execution *and*
the test execution, whereas your new dg-test-timeout-factor only
affects the test execution. (And still measured on the host.)
Not really, `dg-timeout-factor' is only applied to tool execution and it
doesn't affect test execution. Timeout value reporting used to be limited
in DejaGNU, but you can enable it easily now by adding the DejaGNU patch
series referred in the cover letter and see that `dg-timeout-factor' is
ignored for test execution.
Then we need a better name for this new feature that more clearly
indicates that it applies to running executables compiled as part of a
test. Also, 'test_timeout' is documented as a knob for site
configuration to twiddle, not for testsuites to adjust. I support
adding scale factors for testsuites to indicate "this test takes longer
than usual" but these will need to be thought through. This quick hack
will cause future maintenance problems.
Usually the compilation time is close to 0, so is this based on
an actual need more than an itchy "wart"?
Or did I miss something?
Compilation is usually quite fast, but this is not always the case. If
you look at the tests that do use `dg-timeout-factor' in GCC, and some
commits that added the setting, then you ought to find actual use cases.
I saw at least one such a test that takes an awful lot of time here on a
reasonably fast host machine and still passes where GCC has been built
with optimisation enabled, but does time out in the compilation phase if
the compiler has been built at -O0 for debugging purposes. I'd have to
chase it though if you couldn't find it as I haven't written the name
down.
So yes, `dg-timeout-factor' does have its use, but it is different from
that of `dg-test-timeout-factor', hence the need for a separate setting.
This name has already caused confusion and the patch has not even been
accepted yet. The feature is desirable but this implementation is not
acceptable.
At the moment, there are two blocking issues with this patch:
1. The global variable name 'test_timeout_factor' is not acceptable
because it has already caused confusion, apparently among GCC developers
who should be familiar with the GCC testsuite. If it already confuses
GCC testsuite domain experts, its meaning is too unclear for general
use. While looking for alternative names, I found the fundamental
problem with this proposed implementation: test phases (such as running
a test program versus running the tool itself) are defined by the
testsuite, not by the framework. DejaGnu therefore cannot explicitly
support this as offered because the proposal violates encapsulation both
ways.
2. New code in DejaGnu using expr(n) is to have the expression braced
as recommended in the expr(n) manpage, unless it actually uses the
semantics provided by unbraced expr expressions, in which case it
*needs* a comment explaining and justifying that.
The second issue is trivially fixable, but the first appears fatal.
There is a new "testcase" mulitplex command in Git master, which will be
included in the next release, that is intended for testsuites to express
dynamic state. The original planned use was to support hierarchical
test groups, for which a "testcase group" command is currently defined.
In the future, dg.exp will be extended to use "testcase group" to
delimit each testcase that it processes, and the framework will itself
explicitly track each test script as a group. (DejaGnu's current
semantics implicitly group tests by test scripts, but only by (*.exp)
scripts.) Could this multiplex be a suitable place to put this API feature?
Using a command also has the advantage that it will cause a hard failure
if the framework does not implement it, unlike a variable that a test
script can set for the framework to silently ignore, leading to
hard-to-reproduce test (timeout) failures if an older framework is used
with a testsuite expecting this feature. The semantics of "testcase
patience" or similar would be defined to extend to the end of the group
(or test script in versions of DejaGnu that do not fully implement
groups) in which it is executed. This limited scope is needed because
allowing timeout scale factors to "bleed over" to the next test
