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
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