[SPARK-29571][SQL][TESTS][FOLLOWUP] Fix UT in AllExecutionsPageSuite #26234
Conversation
…lass ###What changes were proposed in this pull request? This is a follow-up of apache#24052 to correct assert condition. ###Why are the changes needed? To test IllegalArgumentException condition.. ###Does this PR introduce any user-facing change? No. ###How was this patch tested? Manual Test (during fixing of SPARK-29453 find this issue) Author: 07ARB <[email protected]>
|
@HeartSaVioR , Please check this two statement in AllExecutionsPageSuite.scala Testname : "sorting should be successful" |
|
Yeah I know what this patch is fixing, so left +1 (non-binding). The change won't make existing test fail, right? |
yes |
|
cc @shahidki31 |
|
ok to test |
|
Hi, @07ARB . Thank you for splitting this. |
dongjoon-hyun
changed the title
dongjoon-hyun
changed the title
@HeartSaVioR, @dongjoon-hyun ,@shahidki31 Thank you |
|
Test build #112575 has finished for PR 26234 at commit
|
|
Merged to master. |
| when(request.getParameterMap()).thenReturn(map) | ||
| val html = renderSQLPage(request, tab, statusStore).toString().toLowerCase(Locale.ROOT) | ||
| assert(!html.contains("IllegalArgumentException")) | ||
| assert(!html.contains("illegalargumentexception")) |
There was a problem hiding this comment.
BTW, how come the tests passed before, @07ARB?
There was a problem hiding this comment.
So the problem was that the assertion effectively doesn't verify anything - finding upper&lower string from lowercased string.
8 participants
What changes were proposed in this pull request?
This is a follow-up of #24052 to correct assert condition.
Why are the changes needed?
To test IllegalArgumentException condition..
Does this PR introduce any user-facing change?
No.
How was this patch tested?
Manual Test (during fixing of SPARK-29453 find this issue)