Add validation to the usage service #54617
Conversation
Today the usage service can let in some issues, such as handlers that do not have a name, where the errors do not manifest until later (calling the usage API), or conflicting handlers with the same name. This commit addresses this by adding some validation to the usage service.
jasontedor
added
>enhancement
:Core/Infra/REST API
|
Pinging @elastic/es-core-infra (:Core/Infra/REST API) |
| @@ -41,6 +42,45 @@ | |||
|
|
|||
| public class UsageServiceTests extends ESTestCase { | |||
|
|
|||
| public void testNullHandler() { | |||
There was a problem hiding this comment.
How do you feel about adding JavaDoc for these new test cases? We added something to the contribution guidelines about this, and I personally find it enormously useful to check that the stated purpose of the test matches the implementation.
There was a problem hiding this comment.
Also, and maybe this is just a preference, but I like to name my tests so that they contain the expectation - so testHandlerCannotBeNull, or testHandlerMustHaveAName, etc. Otherwise the test name says what the test is doing, but not what the outcome should be. There's overlap with the JavaDoc, but it might be useful in e.g. the Gradle UI.
There was a problem hiding this comment.
I added Javadocs, and I adjusted the name of testHandlerCannotBeNull, but testAHandlerWithNoName I'm keeping because it's a reference to a song, and it's fun.
|
|
||
| public void testAHandlerWithNoName() { | ||
| final UsageService service = new UsageService(); | ||
| final BaseRestHandler horse = new MockRestHandler(null); |
There was a problem hiding this comment.
Quality variable name! 👍🐴
There was a problem hiding this comment.
Little slice of American pie right there. 😉
| public void testHandlerWithConflictingNamesButSameInstance() { | ||
| final UsageService service = new UsageService(); | ||
| final String name = randomAlphaOfLength(8); | ||
| final BaseRestHandler first = new MockRestHandler(name); | ||
| service.addRestHandler(first); | ||
| // nothing bad should happen | ||
| // nothing bad ever happens to me |
Today the usage service can let in some issues, such as handlers that do not have a name, where the errors do not manifest until later (calling the usage API), or conflicting handlers with the same name. This commit addresses this by adding some validation to the usage service.
|
Nice work! |
Today the usage service can let in some issues, such as handlers that do not have a name, where the errors do not manifest until later (calling the usage API), or conflicting handlers with the same name. This commit addresses this by adding some validation to the usage service.