* [PATCH] diagnostics: Fix up selftests with $COLUMNS < 42 [PR108973]
@ 2023-03-03 9:35 Jakub Jelinek
2023-03-03 13:54 ` David Malcolm
0 siblings, 1 reply; 3+ messages in thread
From: Jakub Jelinek @ 2023-03-03 9:35 UTC (permalink / raw)
To: David Malcolm; +Cc: gcc-patches
Hi!
As mentioned in the PR, GCC's diagnostics self-tests fail if $COLUMNS < 42.
Guarding each self-test with if (get_terminal_width () > 41) or similar
would be a maintainance nightmare (PR has a patch to do so without
reformatting to make it work for $COLUMNS in [30, 41] inclusive, but
I'm afraid going down to $COLUMNS 1 would mean marking everything).
Furthermore, the self-tests don't really emit stuff to the terminal,
but into a buffer, so using get_terminal_width () for it seems
inappropriate. The following patch makes sure test_diagnostic_context
constructor uses at least 80 columns wide caret max width, of course
some tests override it already if they want to test for behavior in narrower
cases.
Bootstrapped/regtested on x86_64-linux and i686-linux, plus tested
on self-tests with $COLUMNS down to 1, ok for trunk?
2023-03-03 Jakub Jelinek <jakub@redhat.com>
PR testsuite/108973
* selftest-diagnostic.cc
(test_diagnostic_context::test_diagnostic_context): Ensure
caret_max_width isn't smaller than 80.
--- gcc/selftest-diagnostic.cc.jj 2023-01-02 09:32:31.991146491 +0100
+++ gcc/selftest-diagnostic.cc 2023-03-02 10:05:17.974321025 +0100
@@ -41,6 +41,7 @@ test_diagnostic_context::test_diagnostic
show_column = true;
start_span = start_span_cb;
min_margin_width = 6;
+ caret_max_width = MAX (caret_max_width, 80);
}
test_diagnostic_context::~test_diagnostic_context ()
Jakub
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] diagnostics: Fix up selftests with $COLUMNS < 42 [PR108973]
2023-03-03 9:35 [PATCH] diagnostics: Fix up selftests with $COLUMNS < 42 [PR108973] Jakub Jelinek
@ 2023-03-03 13:54 ` David Malcolm
2023-03-04 8:50 ` [committed] diagnostics, v2: " Jakub Jelinek
0 siblings, 1 reply; 3+ messages in thread
From: David Malcolm @ 2023-03-03 13:54 UTC (permalink / raw)
To: Jakub Jelinek; +Cc: gcc-patches
On Fri, 2023-03-03 at 10:35 +0100, Jakub Jelinek wrote:
> Hi!
>
> As mentioned in the PR, GCC's diagnostics self-tests fail if $COLUMNS
> < 42.
> Guarding each self-test with if (get_terminal_width () > 41) or
> similar
> would be a maintainance nightmare (PR has a patch to do so without
> reformatting to make it work for $COLUMNS in [30, 41] inclusive, but
> I'm afraid going down to $COLUMNS 1 would mean marking everything).
> Furthermore, the self-tests don't really emit stuff to the terminal,
> but into a buffer, so using get_terminal_width () for it seems
> inappropriate. The following patch makes sure
> test_diagnostic_context
> constructor uses at least 80 columns wide caret max width, of course
> some tests override it already if they want to test for behavior in
> narrower
> cases.
>
> Bootstrapped/regtested on x86_64-linux and i686-linux, plus tested
> on self-tests with $COLUMNS down to 1, ok for trunk?
>
> 2023-03-03 Jakub Jelinek <jakub@redhat.com>
>
> PR testsuite/108973
> * selftest-diagnostic.cc
> (test_diagnostic_context::test_diagnostic_context): Ensure
> caret_max_width isn't smaller than 80.
>
> --- gcc/selftest-diagnostic.cc.jj 2023-01-02 09:32:31.991146491
> +0100
> +++ gcc/selftest-diagnostic.cc 2023-03-02 10:05:17.974321025 +0100
> @@ -41,6 +41,7 @@ test_diagnostic_context::test_diagnostic
> show_column = true;
> start_span = start_span_cb;
> min_margin_width = 6;
> + caret_max_width = MAX (caret_max_width, 80);
> }
Thanks for working on this.
Patch is OK, but I wonder if it would even better to just hardcode
caret_max_width as 80 here, to better eliminate that influence from
from the environment in the unit tests? I think all of the tests that
modify caret_max_width do so after the ctor has run.
Dave
^ permalink raw reply [flat|nested] 3+ messages in thread
* [committed] diagnostics, v2: Fix up selftests with $COLUMNS < 42 [PR108973]
2023-03-03 13:54 ` David Malcolm
@ 2023-03-04 8:50 ` Jakub Jelinek
0 siblings, 0 replies; 3+ messages in thread
From: Jakub Jelinek @ 2023-03-04 8:50 UTC (permalink / raw)
To: David Malcolm; +Cc: gcc-patches
On Fri, Mar 03, 2023 at 08:54:32AM -0500, David Malcolm wrote:
> Thanks for working on this.
>
> Patch is OK, but I wonder if it would even better to just hardcode
> caret_max_width as 80 here, to better eliminate that influence from
> from the environment in the unit tests? I think all of the tests that
> modify caret_max_width do so after the ctor has run.
Thanks, here is what I've committed after another bootstrap/regtest:
2023-03-04 Jakub Jelinek <jakub@redhat.com>
PR testsuite/108973
* selftest-diagnostic.cc
(test_diagnostic_context::test_diagnostic_context): Set
caret_max_width to 80.
--- gcc/selftest-diagnostic.cc.jj 2023-01-02 09:32:31.991146491 +0100
+++ gcc/selftest-diagnostic.cc 2023-03-02 10:05:17.974321025 +0100
@@ -41,6 +41,7 @@ test_diagnostic_context::test_diagnostic
show_column = true;
start_span = start_span_cb;
min_margin_width = 6;
+ caret_max_width = 80;
}
test_diagnostic_context::~test_diagnostic_context ()
Jakub
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2023-03-04 8:50 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-03-03 9:35 [PATCH] diagnostics: Fix up selftests with $COLUMNS < 42 [PR108973] Jakub Jelinek
2023-03-03 13:54 ` David Malcolm
2023-03-04 8:50 ` [committed] diagnostics, v2: " Jakub Jelinek
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).