NextbrickInc commented on PR #4818:
URL: https://github.com/apache/solr/pull/4818#issuecomment-5437537222

   Thanks for going through it, @janhoy.
   
   On the packaging point — fair, and I would have split it if I could have. 
The five came out of one run: they are what the new Selenium suite reported the 
first time it ran without exemptions. Three of them are coupled through a 
single file, because `AdminUiSchemaDesignerTest`'s `@AwaitsFix` and 
`AdminUiTestBase`'s console-error filters were what had been hiding them, and 
removing those is what surfaced the failures. Splitting after the fact would 
leave a window where the suite is green only because the exemptions are still 
in place. If you would still rather have them apart, the clean seam is 
**`PingRequestHandler` + Core Overview** (defect 4, the only server-side 
change) against **the four Admin UI-only fixes** — say the word and I will 
carve it that way.
   
   I have answered all three inline comments. One of them turned into a change: 
9b5545ddb4 makes the ping status handling in `core-overview.js` name its three 
known states explicitly, so an unknown value no longer renders an "enable ping" 
control that could not work. Behaviour for `enabled`, `disabled` and 
`not_configured` is unchanged.
   
   Validation for that commit: the full Admin UI Selenium suite with 
`-Ptests.selenium=true` against headless Chrome — 53 tests across 18 suites, 0 
failures, 0 skipped.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to