added a comment - - edited
Assigned to Andrea for advice:
Oh, my. What a bag of bugs. Where do I start?
(1) My original patch is ineffective. This is not a race condition that causes the test to fail. It is the race condition that causes the saved users.properties to be read from disk and successfully reveal that the underlying code to save the file is broken. The test is broken and does not test anything unless the race condition occurs.
(2) GeoserverUserDao.reload() has no effect at all, because it nulls out userDefinitionsFile, but because userMap is already set, checkUserMap() never has any effect. Nulling out userMap in GeoserverUserDao.reload() causes NewUserMapTest to fail every time, removing the race condition by demonstrating just how broken the underlying code is.
I have a patch for GeoserverUserDao that fixes the reload() logic, but this method is only used in testFill().
The underlying problem is that the test user is written with no authorities, so in the rare (race condition) event that the user.properties file is read, that line is ignored, because an acegi UserAttribute with zero authorities is invalid, and the test fails.
Can we just remove this test method [NewUserPageTest.testFill()]? The comments suggest that the palette behaviour prevents authority testing. The test tests nothing because GeoserverUserDao.reload() is broken, and even if fixed, the palette behaviour makes NewUserPageTest.testFill() fail.