Thanks for the response Mike… So I finished up going through the test code, and yeah, wow…. Doing it one file at a time was educational at least ;-).
https://github.com/apache/solr/compare/main...epugh:intellij_inspired_cleanups?expand=1 In terms of a workflow, should I open up a single JIRA and then list under it a task for each type of fix? And then merge each individual type of fix? So a single JIRA issue “Examine IntelliJ Warnings in Test Code”, and then a JIRA under that for each type, starting with “Remove Exceptions not thrown by Method”? Then merge each one to main, wait a few days to make sure no spike in errors, and then do the next one? What about back porting, would you want these back ported to 8 and 9? Or just 9? Eric > On May 27, 2022, at 8:50 PM, Mike Drob <md...@mdrob.com> wrote: > > Declaring an unused thrown exception in tests isn't the most critical change, > but cleaning this up might help us discover accidental API signature changes > in the future. If a test throws an exception then JUnit will figure it out > and fail the test anyway, which is probably what we want to do regardless. > > I'm +0 on this, like I am not going to go out of my way to refactor that, but > now that you've done it I don't want to just throw away your work so it's > probably fine to commit. I hope this was some automated fix you could apply > instead of doing manually. > But I'm also not going to review it, so I hope you trust the automated > tooling and are willing to volunteer watching Jenkins for a few days after. :) > > Unused exceptions anywhere under src/main I would be _very_ interested in, on > the other hand. > > Mike > > On Fri, May 27, 2022 at 7:33 PM Eric Pugh <ep...@opensourceconnections.com > <mailto:ep...@opensourceconnections.com>> wrote: > So, going through and cleaning up unused throwing of exceptions, I’ve touched > all these files listed below. I was thinking I would do ONE commit for all > of the “remove unused Exception”…. Before I keep going, wanted to make sure > that makes sense….. > > modified: > solr/core/src/test/org/apache/solr/analysis/CommonGramsPhraseQueryTest.java > modified: > solr/core/src/test/org/apache/solr/analysis/PathHierarchyTokenizerFactoryTest.java > modified: > solr/core/src/test/org/apache/solr/analysis/TestLuceneMatchVersion.java > modified: > solr/core/src/test/org/apache/solr/analysis/TestReversedWildcardFilterFactory.java > modified: > solr/core/src/test/org/apache/solr/analysis/TokenizerChainTest.java > modified: > solr/core/src/test/org/apache/solr/cloud/ActionThrottleTest.java > modified: > solr/core/src/test/org/apache/solr/cloud/AssignBackwardCompatibilityTest.java > modified: > solr/core/src/test/org/apache/solr/cloud/ChaosMonkeyShardSplitTest.java > modified: > solr/core/src/test/org/apache/solr/cloud/CollectionPropsTest.java > modified: > solr/core/src/test/org/apache/solr/cloud/CollectionsAPISolrJTest.java > modified: > solr/core/src/test/org/apache/solr/cloud/ConcurrentCreateRoutedAliasTest.java > modified: > solr/core/src/test/org/apache/solr/cloud/ConfigSetApiLockingTest.java > modified: > solr/core/src/test/org/apache/solr/cloud/CreateRoutedAliasTest.java > modified: > solr/core/src/test/org/apache/solr/cloud/DeleteShardTest.java > modified: > solr/core/src/test/org/apache/solr/cloud/DistribJoinFromCollectionTest.java > modified: > solr/core/src/test/org/apache/solr/cloud/ForceLeaderTest.java > modified: > solr/core/src/test/org/apache/solr/cloud/HttpPartitionTest.java > modified: > solr/core/src/test/org/apache/solr/cloud/LeaderElectionTest.java > modified: solr/core/src/test/org/apache/solr/cloud/OverseerTest.java > modified: > solr/core/src/test/org/apache/solr/cloud/ReindexCollectionTest.java > modified: > solr/core/src/test/org/apache/solr/cloud/SSLMigrationTest.java > modified: > solr/core/src/test/org/apache/solr/cloud/SolrCLIZkUtilsTest.java > modified: > solr/core/src/test/org/apache/solr/cloud/TestAuthenticationFramework.java > modified: > solr/core/src/test/org/apache/solr/cloud/TestBaseStatsCacheCloud.java > modified: > solr/core/src/test/org/apache/solr/cloud/TestCloudDeleteByQuery.java > modified: > solr/core/src/test/org/apache/solr/cloud/TestCloudInspectUtil.java > modified: > solr/core/src/test/org/apache/solr/cloud/TestCloudPivotFacet.java > modified: > solr/core/src/test/org/apache/solr/cloud/TestHashPartitioner.java > modified: > solr/core/src/test/org/apache/solr/cloud/TestPrepRecovery.java > modified: > solr/core/src/test/org/apache/solr/cloud/TestRebalanceLeaders.java > modified: > solr/core/src/test/org/apache/solr/cloud/TestSSLRandomization.java > modified: > solr/core/src/test/org/apache/solr/cloud/TestStressCloudBlindAtomicUpdates.java > modified: > solr/core/src/test/org/apache/solr/cloud/TestTlogReplica.java > modified: solr/core/src/test/org/apache/solr/cloud/ZkCLITest.java > modified: > solr/core/src/test/org/apache/solr/cloud/api/collections/AsyncCallRequestStatusResponseTest.java > modified: > solr/core/src/test/org/apache/solr/cloud/api/collections/BackupRestoreApiErrorConditionsTest.java > modified: > solr/core/src/test/org/apache/solr/cloud/api/collections/CollectionApiLockingTest.java > modified: > solr/core/src/test/org/apache/solr/cloud/api/collections/CollectionTooManyReplicasTest.java > modified: > solr/core/src/test/org/apache/solr/cloud/api/collections/ReplicaPropertiesBase.java > modified: > solr/core/src/test/org/apache/solr/cloud/api/collections/TestLocalFSCloudBackupRestore.java > modified: > solr/core/src/test/org/apache/solr/cloud/api/collections/TestReplicaProperties.java > modified: > solr/core/src/test/org/apache/solr/cluster/events/impl/CollectionsRepairEventListenerTest.java > modified: > solr/core/src/test/org/apache/solr/core/AlternateDirectoryTest.java > modified: > solr/core/src/test/org/apache/solr/core/ConfigureRecoveryStrategyTest.java > modified: > solr/core/src/test/org/apache/solr/core/DirectoryFactoryTest.java > modified: solr/core/src/test/org/apache/solr/core/HelloStream.java > modified: > solr/core/src/test/org/apache/solr/core/ResourceLoaderTest.java > modified: solr/core/src/test/org/apache/solr/core/SOLR749Test.java > modified: solr/core/src/test/org/apache/solr/core/SolrCoreTest.java > modified: > solr/core/src/test/org/apache/solr/core/TestBackupRepositoryFactory.java > modified: > solr/core/src/test/org/apache/solr/core/TestCodecSupport.java > modified: solr/core/src/test/org/apache/solr/core/TestConfig.java > modified: > solr/core/src/test/org/apache/solr/core/TestConfigSetImmutable.java > modified: > solr/core/src/test/org/apache/solr/core/TestConfigSetProperties.java > modified: solr/core/src/test/org/apache/solr/core/TestConfigSets.java > modified: > solr/core/src/test/org/apache/solr/core/TestShardHandlerFactory.java > modified: > solr/core/src/test/org/apache/solr/core/TestXIncludeConfig.java > modified: > solr/core/src/test/org/apache/solr/core/snapshots/TestSolrCloudSnapshots.java > modified: > solr/core/src/test/org/apache/solr/core/snapshots/TestSolrCoreSnapshots.java > modified: > solr/core/src/test/org/apache/solr/handler/DocumentAnalysisRequestHandlerTest.java > modified: > solr/core/src/test/org/apache/solr/handler/FieldAnalysisRequestHandlerTest.java > modified: > solr/core/src/test/org/apache/solr/handler/MoreLikeThisHandlerTest.java > modified: > solr/core/src/test/org/apache/solr/handler/PingRequestHandlerTest.java > modified: > solr/core/src/test/org/apache/solr/handler/SearchHandlerTest.java > modified: > solr/core/src/test/org/apache/solr/handler/TestContainerPlugin.java > modified: > solr/core/src/test/org/apache/solr/handler/TestIncrementalCoreBackup.java > modified: > solr/core/src/test/org/apache/solr/handler/TestSnapshotCoreBackup.java > modified: > solr/core/src/test/org/apache/solr/handler/TestSolrConfigHandlerCloud.java > modified: > solr/core/src/test/org/apache/solr/handler/V2ApiIntegrationTest.java > modified: > solr/core/src/test/org/apache/solr/handler/V2ClusterAPIMappingTest.java > modified: > solr/core/src/test/org/apache/solr/handler/V2UpdateAPIMappingTest.java > modified: > solr/core/src/test/org/apache/solr/handler/XmlUpdateRequestHandlerTest.java > modified: > solr/core/src/test/org/apache/solr/handler/admin/CoreAdminOperationTest.java > modified: > solr/core/src/test/org/apache/solr/handler/admin/CoreMergeIndexesAdminHandlerTest.java > modified: > solr/core/src/test/org/apache/solr/handler/admin/HealthCheckHandlerTest.java > modified: > solr/core/src/test/org/apache/solr/handler/admin/IndexSizeEstimatorTest.java > modified: > solr/core/src/test/org/apache/solr/handler/admin/LukeRequestHandlerTest.java > modified: > solr/core/src/test/org/apache/solr/handler/admin/MetricsHandlerTest.java > modified: > solr/core/src/test/org/apache/solr/handler/admin/ShowFileRequestHandlerTest.java > modified: > solr/core/src/test/org/apache/solr/handler/admin/SplitHandlerTest.java > modified: > solr/core/src/test/org/apache/solr/handler/admin/StatsReloadRaceTest.java > modified: > solr/core/src/test/org/apache/solr/handler/admin/SystemInfoHandlerTest.java > modified: > solr/core/src/test/org/apache/solr/handler/admin/TestApiFramework.java > modified: > solr/core/src/test/org/apache/solr/handler/admin/TestCollectionAPIs.java > modified: > solr/core/src/test/org/apache/solr/handler/admin/V2CollectionBackupsAPIMappingTest.java > modified: > solr/core/src/test/org/apache/solr/handler/admin/V2CollectionsAPIMappingTest.java > modified: > solr/core/src/test/org/apache/solr/handler/admin/api/V2CollectionAPIMappingTest.java > modified: > solr/core/src/test/org/apache/solr/handler/admin/api/V2CoreAPIMappingTest.java > modified: > solr/core/src/test/org/apache/solr/handler/admin/api/V2NodeAPIMappingTest.java > modified: > solr/core/src/test/org/apache/solr/handler/admin/api/V2ShardsAPIMappingTest.java > modified: > solr/core/src/test/org/apache/solr/handler/component/BadComponentTest.java > modified: > solr/core/src/test/org/apache/solr/handler/component/CustomHighlightComponentTest.java > modified: > solr/core/src/test/org/apache/solr/handler/component/DebugComponentTest.java > modified: > solr/core/src/test/org/apache/solr/handler/component/DistributedExpandComponentTest.java > modified: > solr/core/src/test/org/apache/solr/handler/component/DistributedFacetExistsSmallTest.java > modified: > solr/core/src/test/org/apache/solr/handler/component/DistributedSpellCheckComponentTest.java > modified: > solr/core/src/test/org/apache/solr/handler/component/DistributedSuggestComponentTest.java > modified: > solr/core/src/test/org/apache/solr/handler/component/FacetPivotSmallTest.java > modified: > solr/core/src/test/org/apache/solr/handler/component/ShardRequestTest.java > modified: > solr/core/src/test/org/apache/solr/handler/component/SpellCheckComponentTest.java > modified: > solr/core/src/test/org/apache/solr/handler/component/SuggestComponentContextFilterQueryTest.java > modified: > solr/core/src/test/org/apache/solr/handler/component/SuggestComponentTest.java > modified: > solr/core/src/test/org/apache/solr/handler/component/TestExpandComponent.java > modified: > solr/core/src/test/org/apache/solr/handler/component/TestHttpShardHandlerFactory.java > modified: > solr/core/src/test/org/apache/solr/handler/component/TestPivotHelperCode.java > modified: > solr/core/src/test/org/apache/solr/handler/loader/JavabinLoaderTest.java > modified: > solr/core/src/test/org/apache/solr/handler/tagger/EmbeddedSolrNoSerializeTest.java > modified: > solr/core/src/test/org/apache/solr/handler/tagger/XmlInterpolationTest.java > modified: > solr/core/src/test/org/apache/solr/highlight/DummyHighlighter.java > modified: > solr/core/src/test/org/apache/solr/highlight/HighlighterMaxOffsetTest.java > modified: > solr/core/src/test/org/apache/solr/highlight/HighlighterTest.java > modified: > solr/core/src/test/org/apache/solr/internal/csv/CharBufferTest.java > modified: > solr/core/src/test/org/apache/solr/legacy/TestLegacyField.java > modified: > solr/core/src/test/org/apache/solr/legacy/TestLegacyNumericUtils.java > modified: > solr/core/src/test/org/apache/solr/metrics/JvmMetricsTest.java > modified: > solr/core/src/test/org/apache/solr/metrics/MetricsConfigTest.java > modified: > solr/core/src/test/org/apache/solr/metrics/SolrCoreMetricManagerTest.java > modified: > solr/core/src/test/org/apache/solr/metrics/SolrMetricManagerTest.java > modified: > solr/core/src/test/org/apache/solr/metrics/reporters/SolrJmxReporterTest.java > modified: > solr/core/src/test/org/apache/solr/metrics/reporters/SolrSlf4jReporterTest.java > modified: solr/core/src/test/org/apache/solr/request/TestFaceting.java > modified: > solr/core/src/test/org/apache/solr/response/TestJavabinTupleStreamParser.java > modified: > solr/core/src/test/org/apache/solr/response/TestRawResponseWriter.java > modified: > solr/core/src/test/org/apache/solr/response/TestRetrieveFieldsOptimizer.java > modified: > solr/core/src/test/org/apache/solr/response/TestSolrQueryResponse.java > modified: > solr/core/src/test/org/apache/solr/response/transform/TestChildDocTransformer.java > modified: > solr/core/src/test/org/apache/solr/response/transform/TestChildDocTransformerHierarchy.java > modified: > solr/core/src/test/org/apache/solr/response/transform/TestExplainDocTransformer.java > modified: > solr/core/src/test/org/apache/solr/response/transform/TestSubQueryTransformer.java > modified: > solr/core/src/test/org/apache/solr/response/transform/TestSubQueryTransformerCrossCore.java > modified: > solr/core/src/test/org/apache/solr/rest/TestManagedResource.java > modified: solr/core/src/test/org/apache/solr/rest/TestRestManager.java > modified: > solr/core/src/test/org/apache/solr/rest/schema/TestDynamicFieldCollectionResource.java > modified: > solr/core/src/test/org/apache/solr/rest/schema/TestDynamicFieldResource.java > modified: > solr/core/src/test/org/apache/solr/rest/schema/TestFieldCollectionResource.java > modified: > solr/core/src/test/org/apache/solr/rest/schema/TestFieldResource.java > modified: > solr/core/src/test/org/apache/solr/rest/schema/TestFieldTypeCollectionResource.java > modified: > solr/core/src/test/org/apache/solr/rest/schema/TestFieldTypeResource.java > modified: > solr/core/src/test/org/apache/solr/rest/schema/TestSchemaNameResource.java > modified: > solr/core/src/test/org/apache/solr/rest/schema/TestSchemaResource.java > modified: > solr/core/src/test/org/apache/solr/rest/schema/TestSchemaSimilarityResource.java > modified: > solr/core/src/test/org/apache/solr/rest/schema/TestSchemaVersionResource.java > modified: > solr/core/src/test/org/apache/solr/rest/schema/TestSerializedLuceneMatchVersion.java > modified: > solr/core/src/test/org/apache/solr/rest/schema/TestUniqueKeyFieldResource.java > modified: > solr/core/src/test/org/apache/solr/schema/CurrencyFieldTypeTest.java > modified: > solr/core/src/test/org/apache/solr/schema/DocValuesMultiTest.java > modified: solr/core/src/test/org/apache/solr/schema/EnumFieldTest.java > modified: > solr/core/src/test/org/apache/solr/schema/ExternalFileFieldSortTest.java > modified: > solr/core/src/test/org/apache/solr/schema/IndexSchemaTest.java > modified: > solr/core/src/test/org/apache/solr/schema/ManagedSchemaRoundRobinCloudTest.java > modified: solr/core/src/test/org/apache/solr/schema/PolyFieldTest.java > modified: > solr/core/src/test/org/apache/solr/schema/PreAnalyzedFieldTest.java > modified: solr/core/src/test/org/apache/solr/schema/RankFieldTest.java > modified: > solr/core/src/test/org/apache/solr/schema/SchemaWatcherTest.java > modified: > solr/core/src/test/org/apache/solr/schema/SpatialRPTFieldTypeTest.java > modified: > solr/core/src/test/org/apache/solr/schema/TestCloudManagedSchema.java > modified: > solr/core/src/test/org/apache/solr/schema/TestManagedSchema.java > modified: > solr/core/src/test/org/apache/solr/schema/TestSortableTextField.java > modified: > solr/core/src/test/org/apache/solr/schema/TestUseDocValuesAsStored.java > > >> On May 27, 2022, at 1:52 PM, David Smiley <dsmi...@apache.org >> <mailto:dsmi...@apache.org>> wrote: >> >> IntelliJ is produced by a company and I have no idea how they go about >> selecting what the default inspections (what IntelliJ calls these) are. >> Maybe it was one person there, maybe it was arbitrary by whoever wrote the >> inspection, or maybe they had some more thoughtful approach that looked at >> literature. Regardless, I disagree with some of their choices. I think we >> should base our decisions on what inspections to address for ourselves, not >> just because JetBrains included them. I routinely adjust my IntelliJ >> inspection settings to not harass me about some matters that I consider to >> be frivolous. For example boolean expression simplifications -- where we as >> a project (when a part of Lucene) have chosen "== false" to be clearer than >> an exclamation point adjacent to a boolean expression. >> >> If we do some of this: Agreed on picking exactly one "inspection" and >> scoping to just one module at first. Could increase to more commits in the >> same PR if you get good feedback. >> Personally, I wouldn't do this endeavor unless the particular inspection is >> something that particularly motivates me / was a pet-peeve. >> I think "getting to green" is a toal lost cause unless we were to enforce a >> particular configured list of inspections (which is IntelliJ only, remember). >> >> ~ David Smiley >> Apache Lucene/Solr Search Developer >> http://www.linkedin.com/in/davidwsmiley >> <http://www.linkedin.com/in/davidwsmiley> >> >> On Fri, May 27, 2022 at 12:52 PM Shawn Heisey <apa...@elyograg.org >> <mailto:apa...@elyograg.org>> wrote: >> On 5/27/2022 8:24 AM, Eric Pugh wrote: >> > Hey all, was poking around at a unit test while watching TV and >> > noticed lots of warnings from IntelliJ, little stuff like exceptions >> > being thrown that don’t need to be thrown, unused variables, or typos. >> >> In eclipse, there are THOUSANDS of warnings. And last I checked, even a >> bunch of errors. But I was able to build 10.0.0-SNAPSHOT successfully. >> >> Thanks, >> Shawn >> >> >> --------------------------------------------------------------------- >> To unsubscribe, e-mail: dev-unsubscr...@solr.apache.org >> <mailto:dev-unsubscr...@solr.apache.org> >> For additional commands, e-mail: dev-h...@solr.apache.org >> <mailto:dev-h...@solr.apache.org> >> > > _______________________ > Eric Pugh | Founder & CEO | OpenSource Connections, LLC | 434.466.1467 | > http://www.opensourceconnections.com <http://www.opensourceconnections.com/> > | My Free/Busy <http://tinyurl.com/eric-cal> > Co-Author: Apache Solr Enterprise Search Server, 3rd Ed > <https://www.packtpub.com/big-data-and-business-intelligence/apache-solr-enterprise-search-server-third-edition-raw> > > This e-mail and all contents, including attachments, is considered to be > Company Confidential unless explicitly stated otherwise, regardless of > whether attachments are marked as such. > _______________________ Eric Pugh | Founder & CEO | OpenSource Connections, LLC | 434.466.1467 | http://www.opensourceconnections.com <http://www.opensourceconnections.com/> | My Free/Busy <http://tinyurl.com/eric-cal> Co-Author: Apache Solr Enterprise Search Server, 3rd Ed <https://www.packtpub.com/big-data-and-business-intelligence/apache-solr-enterprise-search-server-third-edition-raw> This e-mail and all contents, including attachments, is considered to be Company Confidential unless explicitly stated otherwise, regardless of whether attachments are marked as such.
