ctubbsii commented on code in PR #3445:
URL: https://github.com/apache/accumulo/pull/3445#discussion_r1224494889
##########
test/src/main/java/org/apache/accumulo/test/start/KeywordStartIT.java:
##########
@@ -201,6 +203,8 @@ public void checkHasMain() {
expectSet.add(Shell.class);
expectSet.add(SimpleGarbageCollector.class);
expectSet.add(TabletServer.class);
+ expectSet.add(ZooInfoViewer.class);
+ expectSet.add(ZooPropSetTool.class);
Review Comment:
Tools typically don't have a main method. Everything generally gets launched
through our Start entry point. However, many existing tools happened to have a
main method. So, the test was an attempt to make sure we didn't accidentally
remove a main method that users might be using in scripts.
Other than Start, and tools that don't have a keyword, we don't have a
requirement for any tools to have a "main" method at all. I think you updated
the IT correctly.
This is out of scope for this PR, but we may want to add a test to check
that we don't introduce new main classes unintentionally. Currently we only
have a test that checks that we're preserving the existing ones... but a better
test would be to check through all our classes, and check each one for having a
main method, and failing if it finds any we haven't identified as needing one.
--
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]