Re-enable failing CLI Test #256

coverbeck · Aug 31, 2023

Description
Re-enable failing CLI test by turning off Java assertions in the org.hibernate and subpackages. It also enabled the removal of System.exit catch that is no longer necessary.

This is less than ideal, but note that in prod the web service is not running with assertions enabled.

Notes:

The Surefire plugin by default turns on Java assertions
I was initially unable to reproduce the bug with a local "regular" web service running, because I don't run the web service with assertions enabled. But If I set a breakpoint in the Java code, in org.hibernate.collection.spi.PersistentMap#injectLoadedState, on the assert this.isInitializing(), I can see the assertion would fail if assertions were enabled.
Semi-digression, I can't even start the web service locally with assertions enabled -- it fails with a NoClassDefFound error, unrelated to Hibernate.
By default, running a test in IntelliJ enables assertions, which is why it would fail locally. If I run the test with assertions not enabled locally, it passes.

If this approach is agreed upon, I will create a followup ticket to figure out the underlying assertion failure. I think it's there for optimization purposes, i.e., a call is being made to initialize an already initialized collection:

	public void injectLoadedState(PluralAttributeMapping attributeMapping, List<?> loadingState) {
		assert isInitializing();
		assert map == null;

		final CollectionPersister collectionDescriptor = attributeMapping.getCollectionDescriptor();
		this.map = (Map<K,E>) collectionDescriptor.getCollectionSemantics().instantiateRaw( loadingState.size(), collectionDescriptor );

		for ( int i = 0; i < loadingState.size(); i++ ) {
			final Object[] keyVal = (Object[]) loadingState.get( i );
			map.put( (K) keyVal[0], (E) keyVal[1] );
		}
	}

Review Instructions
Since this is a change to an integration test, if the tests pass after it's merged, it's done, and no further review necessary.

Issue
SEAB-5854

Security
If there are any concerns that require extra attention from the security team, highlight them here.

Please make sure that you've checked the following before submitting your pull request. Thanks!

Check that you pass the basic style checks and unit tests by running ./mvnw clean install
If this PR is for a user-facing feature, create and link a documentation ticket for this feature (usually in the same milestone as the linked issue). Style points if you create a documentation PR directly and link that instead.

sonarqubecloud · Aug 31, 2023

Kudos, SonarCloud Quality Gate passed!

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

No Coverage information
0.0% Duplication

codecov · Aug 31, 2023

Codecov Report

Patch coverage has no change and project coverage change: -0.02% ⚠️

Comparison is base (03ff3ce) 70.26% compared to head (c2b3b0e) 70.24%.

Additional details and impacted files

@@              Coverage Diff              @@
##             develop     #256      +/-   ##
=============================================
- Coverage      70.26%   70.24%   -0.02%     
  Complexity      1052     1052              
=============================================
  Files             47       47              
  Lines           6070     6070              
  Branches         799      799              
=============================================
- Hits            4265     4264       -1     
+ Misses          1467     1466       -1     
- Partials         338      340       +2

Flag	Coverage Δ
bitbuckettests	`9.83% <ø> (ø)`
confidentialtooltests	`55.81% <ø> (ø)`
confidentialworkflowtests	`30.32% <ø> (+0.09%)`	⬆️
nonconfidentialtests	`32.35% <ø> (-0.02%)`	⬇️
singularitytests	`16.58% <ø> (ø)`
unittests	`8.35% <ø> (ø)`

Flags with carried forward coverage won't be shown. Click here to find out more.

see 4 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

coverbeck · Aug 31, 2023

dockstore-cli-integration-testing/src/test/java/io/dockstore/client/cli/WDLWorkflowIT.java


 package io.dockstore.client.cli;

-import java.io.IOException;


There's already a ticket on consistent imports sorting; I think these changes match the web service's layout, but ultimately that ticket should solve.

denis-yuen · Sep 1, 2023

Semi-digression, I can't even start the web service locally with assertions enabled -- it fails with a NoClassDefFound error, unrelated to Hibernate.

🤔 maybe you have a slightly custom classpath?

coverbeck · Sep 1, 2023

Semi-digression, I can't even start the web service locally with assertions enabled -- it fails with a NoClassDefFound error, unrelated to Hibernate.

🤔 maybe you have a slightly custom classpath?

I don't think I do; it should be whatever IntelliJ thinks it is, based on Maven... does it work for you? Here's what I get:

java.lang.NoClassDefFoundError: com/fasterxml/jackson/dataformat/cbor/CBORConstants
	at org.elasticsearch.common.xcontent.XContentFactory.xContentType(XContentFactory.java:313)
	at org.elasticsearch.common.xcontent.XContentHelper.xContentType(XContentHelper.java:427)
	at org.elasticsearch.common.compress.CompressorFactory.compressor(CompressorFactory.java:44)
	at org.elasticsearch.common.compress.CompressedXContent.assertConsistent(CompressedXContent.java:105)
	at org.elasticsearch.common.compress.CompressedXContent.<init>(CompressedXContent.java:84)
	at org.elasticsearch.cluster.metadata.MappingMetadata.<init>(MappingMetadata.java:99)
	at org.elasticsearch.client.indices.GetMappingsResponse.fromXContent(GetMappingsResponse.java:66)
	at org.elasticsearch.client.RestHighLevelClient.parseEntity(RestHighLevelClient.java:1911)
	at org.elasticsearch.client.RestHighLevelClient.lambda$performRequestAndParseEntity$9(RestHighLevelClient.java:1585)
	at org.elasticsearch.client.RestHighLevelClient.internalPerformRequest(RestHighLevelClient.java:1649)
	at org.elasticsearch.client.RestHighLevelClient.performRequest(RestHighLevelClient.java:1617)
	at org.elasticsearch.client.RestHighLevelClient.performRequestAndParseEntity(RestHighLevelClient.java:1584)
	at org.elasticsearch.client.IndicesClient.getMapping(IndicesClient.java:422)
	at io.dockstore.webservice.helpers.ElasticSearchHelper.doMappingsExist(ElasticSearchHelper.java:49)
	at io.dockstore.webservice.DockstoreWebserviceApplication.lambda$run$2(DockstoreWebserviceApplication.java:513)
	at io.dropwizard.lifecycle.setup.LifecycleEnvironment$ServerListener.lifeCycleStarted(LifecycleEnvironment.java:116)
	at org.eclipse.jetty.util.component.AbstractLifeCycle.setStarted(AbstractLifeCycle.java:253)
	at org.eclipse.jetty.util.component.AbstractLifeCycle.start(AbstractLifeCycle.java:94)
	at io.dropwizard.core.cli.ServerCommand.run(ServerCommand.java:52)
	at io.dropwizard.core.cli.EnvironmentCommand.run(EnvironmentCommand.java:67)
	at io.dropwizard.core.cli.ConfiguredCommand.run(ConfiguredCommand.java:98)
	at io.dropwizard.core.cli.Cli.run(Cli.java:78)
	at io.dropwizard.core.Application.run(Application.java:94)
	at io.dockstore.webservice.DockstoreWebserviceApplication.main(DockstoreWebserviceApplication.java:244)
Caused by: java.lang.ClassNotFoundException: com.fasterxml.jackson.dataformat.cbor.CBORConstants
	at java.base/jdk.internal.loader.BuiltinClassLoader.loadClass(BuiltinClassLoader.java:641)
	at java.base/jdk.internal.loader.ClassLoaders$AppClassLoader.loadClass(ClassLoaders.java:188)
	at java.base/java.lang.ClassLoader.loadClass(ClassLoader.java:520)
	... 24 more

denis-yuen · Sep 1, 2023

I don't think I do; it should be whatever IntelliJ thinks it is, based on Maven... does it work for you? Here's what I get:

Oh I see, assertions are not enabled by default for running, just for tests. Yeah, I get the same thing

coverbeck · Sep 1, 2023

@denis-yuen made you an assignee; if no changes are requested by Steve nor Kathy, please go ahead and merge. If changes are requested, I'll respond when I get back from vacation.

coverbeck · Sep 2, 2023

Got another review and merged, never mind.

Charles Overbeck added 7 commits August 25, 2023 17:48

Wild, unfounded guess

a14b19b

WAG 2

aae7e47

Yikes, no medium+

003faab

Turn off assertions for Hibernate packages.

ea1d6bb

Sheesh, one dash

fa9baa1

Unused import

1f1d63b

Sheesh, once again.

3cecc5e

coverbeck self-assigned this Aug 31, 2023

Charles Overbeck added 4 commits August 31, 2023 14:45

Another try

9ac42d7

With assertions turned off, a System.exit no longer occurs.

804d540

One more try

dbb4495

This better be it.

c2b3b0e

coverbeck commented Aug 31, 2023

View reviewed changes

coverbeck requested review from denis-yuen, kathy-t and svonworl August 31, 2023 23:39

denis-yuen approved these changes Sep 1, 2023

View reviewed changes

coverbeck mentioned this pull request Sep 1, 2023

Investigate Hibernate assertion when getting a checker workflow dockstore/dockstore#5654

Open

coverbeck assigned denis-yuen Sep 1, 2023

svonworl approved these changes Sep 2, 2023

View reviewed changes

coverbeck merged commit f1bdd58 into develop Sep 2, 2023

coverbeck deleted the feature/seab-5854/build branch September 2, 2023 00:37

coverbeck unassigned denis-yuen Sep 2, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Re-enable failing CLI Test #256

Re-enable failing CLI Test #256

Uh oh!

coverbeck commented Aug 31, 2023 •

edited

Loading

Uh oh!

sonarqubecloud bot commented Aug 31, 2023

Uh oh!

codecov bot commented Aug 31, 2023 •

edited

Loading

Uh oh!

coverbeck Aug 31, 2023

Uh oh!

denis-yuen commented Sep 1, 2023

Uh oh!

coverbeck commented Sep 1, 2023

Uh oh!

denis-yuen commented Sep 1, 2023

Uh oh!

coverbeck commented Sep 1, 2023

Uh oh!

coverbeck commented Sep 2, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Search code, repositories, users, issues, pull requests...

Re-enable failing CLI Test #256

Re-enable failing CLI Test #256

Uh oh!

Conversation

coverbeck commented Aug 31, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sonarqubecloud bot commented Aug 31, 2023

Uh oh!

codecov bot commented Aug 31, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

coverbeck Aug 31, 2023

Choose a reason for hiding this comment

Uh oh!

denis-yuen commented Sep 1, 2023

Uh oh!

coverbeck commented Sep 1, 2023

Uh oh!

denis-yuen commented Sep 1, 2023

Uh oh!

coverbeck commented Sep 1, 2023

Uh oh!

coverbeck commented Sep 2, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

coverbeck commented Aug 31, 2023 •

edited

Loading

codecov bot commented Aug 31, 2023 •

edited

Loading