ctubbsii commented on PR #2712: URL: https://github.com/apache/accumulo/pull/2712#issuecomment-1132856591
> > Any proposed changes here should include any necessary adjustments to tests that would otherwise be left broken if the changes were accepted. > > Guidance requested. > You'd have to look at each test to see why it's failing. Each test may require something different. It's likely that it's just a trivial fix that is needed... like adding another mocked method call to the set of expected method calls, or implementing a method in a subclass created for testing. > > Okay, it looks like those indicate the problem is likely with using an AccumuloConfiguration object for testing that doesn't override the method. That should never happen in runtime. It seems the tests need to be fixed. > > "That should never happen in runtime. " - Then why was it in the tests? Should those particular tests be rewritten or, considering the tests were for an _impossible_ condition, be deleted outright? Tests often use mock objects to test specific code paths. When you change the behavior of the code, you can break certain assumptions that the test code is relying on. Depending on what changed, it's likely the tests just need minor adjustments. -- 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]
