Re: [Piglit] [PATCH] shader_runner: report PIGLIT_FAIL if linking unexpectedly fails
Quoting Brian Paul (2016-10-18 17:18:55)
> We were previously reporting 'pass' if linking failed.
>
> v2: return the result of program_must_be_in_use(), per Dylan.
> ---
> tests/shaders/shader_runner.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/tests/shaders/shader_runner.c b/tests/shaders/shader_runner.c
> index b0bde2c..598a859 100644
> --- a/tests/shaders/shader_runner.c
> +++ b/tests/shaders/shader_runner.c
> @@ -3528,7 +3528,7 @@ piglit_display(void)
> }
>
> if (!link_ok && !link_error_expected) {
> - program_must_be_in_use();
> + full_result = program_must_be_in_use();
> }
>
> piglit_present_results();
> --
> 1.9.1
>
> ___
> Piglit mailing list
> Piglit@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/piglit
Reviewed-by: Dylan Baker
signature.asc
Description: signature
___
Piglit mailing list
Piglit@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/piglit
Re: [Piglit] [PATCH] shader_runner: report PIGLIT_FAIL if linking unexpectedly fails
Quoting Brian Paul (2016-10-18 17:18:36)
> On 10/18/2016 05:43 PM, Dylan Baker wrote:
> > Quoting Brian Paul (2016-10-18 16:28:53)
> >> We were previously reporting 'pass' if linking failed.
> >> ---
> >> tests/shaders/shader_runner.c | 1 +
> >> 1 file changed, 1 insertion(+)
> >>
> >> diff --git a/tests/shaders/shader_runner.c b/tests/shaders/shader_runner.c
> >> index b0bde2c..94865b7 100644
> >> --- a/tests/shaders/shader_runner.c
> >> +++ b/tests/shaders/shader_runner.c
> >> @@ -3529,6 +3529,7 @@ piglit_display(void)
> >>
> >> if (!link_ok && !link_error_expected) {
> >> program_must_be_in_use();
> >> + full_result = PIGLIT_FAIL;
> >> }
> >>
> >> piglit_present_results();
> >> --
> >> 1.9.1
> >>
> >> ___
> >> Piglit mailing list
> >> Piglit@lists.freedesktop.org
> >> https://lists.freedesktop.org/mailman/listinfo/piglit
> >
> > And, after looking some more, there's a lot of cases where we need to handle
> > this.
>
> Yeah, I saw that and wasn't sure what was intended. My take was
> regardless of what program_must_be_in_use() returns, the fact that if
> link_ok==false and link_error_expected==false it means the test failed.
>
> Changing it to full_result = program_must_be_in_use() works too. I'll
> update the patch.
>
> -Brian
>
That's probably correct. I think either just setting "full_results =
PIGLIT_FAIL" or "full_result = program_must_be_in_use()" is correct, but just
calling the function for no reason then setting it to PIGLIT_FAIL seems a little
strange.
With either of those changes you can add my rb,
Reviewed-by: Dylan Baker
signature.asc
Description: signature
___
Piglit mailing list
Piglit@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/piglit
[Piglit] [PATCH] shader_runner: report PIGLIT_FAIL if linking unexpectedly fails
We were previously reporting 'pass' if linking failed.
v2: return the result of program_must_be_in_use(), per Dylan.
---
tests/shaders/shader_runner.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tests/shaders/shader_runner.c b/tests/shaders/shader_runner.c
index b0bde2c..598a859 100644
--- a/tests/shaders/shader_runner.c
+++ b/tests/shaders/shader_runner.c
@@ -3528,7 +3528,7 @@ piglit_display(void)
}
if (!link_ok && !link_error_expected) {
- program_must_be_in_use();
+ full_result = program_must_be_in_use();
}
piglit_present_results();
--
1.9.1
___
Piglit mailing list
Piglit@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/piglit
Re: [Piglit] [PATCH] shader_runner: report PIGLIT_FAIL if linking unexpectedly fails
On 10/18/2016 05:43 PM, Dylan Baker wrote:
Quoting Brian Paul (2016-10-18 16:28:53)
We were previously reporting 'pass' if linking failed.
---
tests/shaders/shader_runner.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/tests/shaders/shader_runner.c b/tests/shaders/shader_runner.c
index b0bde2c..94865b7 100644
--- a/tests/shaders/shader_runner.c
+++ b/tests/shaders/shader_runner.c
@@ -3529,6 +3529,7 @@ piglit_display(void)
if (!link_ok && !link_error_expected) {
program_must_be_in_use();
+ full_result = PIGLIT_FAIL;
}
piglit_present_results();
--
1.9.1
___
Piglit mailing list
Piglit@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/piglit
And, after looking some more, there's a lot of cases where we need to handle
this.
Yeah, I saw that and wasn't sure what was intended. My take was
regardless of what program_must_be_in_use() returns, the fact that if
link_ok==false and link_error_expected==false it means the test failed.
Changing it to full_result = program_must_be_in_use() works too. I'll
update the patch.
-Brian
___
Piglit mailing list
Piglit@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/piglit
Re: [Piglit] [PATCH] shader_runner: report PIGLIT_FAIL if linking unexpectedly fails
Quoting Brian Paul (2016-10-18 16:28:53)
> We were previously reporting 'pass' if linking failed.
> ---
> tests/shaders/shader_runner.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/tests/shaders/shader_runner.c b/tests/shaders/shader_runner.c
> index b0bde2c..94865b7 100644
> --- a/tests/shaders/shader_runner.c
> +++ b/tests/shaders/shader_runner.c
> @@ -3529,6 +3529,7 @@ piglit_display(void)
>
> if (!link_ok && !link_error_expected) {
> program_must_be_in_use();
> + full_result = PIGLIT_FAIL;
> }
>
> piglit_present_results();
> --
> 1.9.1
>
> ___
> Piglit mailing list
> Piglit@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/piglit
And, after looking some more, there's a lot of cases where we need to handle
this.
Dylan
signature.asc
Description: signature
___
Piglit mailing list
Piglit@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/piglit
Re: [Piglit] [PATCH] shader_runner: report PIGLIT_FAIL if linking unexpectedly fails
Quoting Brian Paul (2016-10-18 16:28:53)
> We were previously reporting 'pass' if linking failed.
> ---
> tests/shaders/shader_runner.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/tests/shaders/shader_runner.c b/tests/shaders/shader_runner.c
> index b0bde2c..94865b7 100644
> --- a/tests/shaders/shader_runner.c
> +++ b/tests/shaders/shader_runner.c
> @@ -3529,6 +3529,7 @@ piglit_display(void)
>
> if (!link_ok && !link_error_expected) {
> program_must_be_in_use();
> + full_result = PIGLIT_FAIL;
That's not quite correct. program_must_be_in_use() was changed to return a
piglit_result (I assume when we did the shader_runner multiple shaders stuff)
Anyway, I think this should be:
full_result = program_must_be_in_use();
Dylan
> }
>
> piglit_present_results();
> --
> 1.9.1
>
> ___
> Piglit mailing list
> Piglit@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/piglit
signature.asc
Description: signature
___
Piglit mailing list
Piglit@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/piglit
[Piglit] [PATCH] shader_runner: report PIGLIT_FAIL if linking unexpectedly fails
We were previously reporting 'pass' if linking failed.
---
tests/shaders/shader_runner.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/tests/shaders/shader_runner.c b/tests/shaders/shader_runner.c
index b0bde2c..94865b7 100644
--- a/tests/shaders/shader_runner.c
+++ b/tests/shaders/shader_runner.c
@@ -3529,6 +3529,7 @@ piglit_display(void)
if (!link_ok && !link_error_expected) {
program_must_be_in_use();
+ full_result = PIGLIT_FAIL;
}
piglit_present_results();
--
1.9.1
___
Piglit mailing list
Piglit@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/piglit
