Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix dependencies on library-manager #12419

Merged
merged 2 commits into from
Mar 5, 2025
Merged

Conversation

hubertp
Copy link
Collaborator

@hubertp hubertp commented Mar 5, 2025

Pull Request Description

This change fixes a dependency on library-manager that was causing ClassNotFoundException when running tests:

[info] LibraryDownloadTest:
[info] DownloadingLibraryCache
SLF4J(I): Attempting to load provider "org.enso.logging.service.logback.test.provider.TestLogProvider" specified via "slf4j.provider" system propertyTests 0s
SLF4J(E): Failed to instantiate the specified SLF4JServiceProvider (org.enso.logging.service.logback.test.provider.TestLogProvider)
SLF4J(E): Reported exception:
java.lang.ClassNotFoundException: org.enso.logging.service.logback.test.provider.TestLogProvider
	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:526)
	at org.slf4j.LoggerFactory.loadExplicitlySpecified(LoggerFactory.java:226)
	at org.slf4j.LoggerFactory.findServiceProviders(LoggerFactory.java:122)

While I was there I noticed that library-manager-test is rather obsolete if only we set the dependencies between projects right.
Hence, classes of library-manage-test are moved to library-manager's test directory and the former is dropped completely.

Important Notes

Other -test projects should be set up in a similar fashion.

Checklist

Please ensure that the following checklist has been satisfied before submitting the PR:

  • All code follows the
    Scala,
    Java,
    TypeScript,
    and
    Rust
    style guides. In case you are using a language not listed above, follow the Rust style guide.

hubertp added 2 commits March 5, 2025 12:20
Only tests should depend on logging setup, for test purposes.
Fixes ClassNotFoundException:
```
[info] LibraryDownloadTest:
[info] DownloadingLibraryCache
SLF4J(I): Attempting to load provider "org.enso.logging.service.logback.test.provider.TestLogProvider" specified via "slf4j.provider" system propertyTests 0s
SLF4J(E): Failed to instantiate the specified SLF4JServiceProvider (org.enso.logging.service.logback.test.provider.TestLogProvider)
SLF4J(E): Reported exception:
java.lang.ClassNotFoundException: org.enso.logging.service.logback.test.provider.TestLogProvider
	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:526)
	at org.slf4j.LoggerFactory.loadExplicitlySpecified(LoggerFactory.java:226)
	at org.slf4j.LoggerFactory.findServiceProviders(LoggerFactory.java:122)
```
Replaced by setting dependencies between projects appropriately using
classifiers.
@hubertp hubertp added the CI: No changelog needed Do not require a changelog entry for this PR. label Mar 5, 2025
@hubertp hubertp requested a review from radeusgd March 5, 2025 16:27
Copy link
Member

@Akirathan Akirathan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I assume this fixes the local sbt library-manager/test command. I also assume that on the CI, everything is fine, since we CI does sbt buildEngineDistribution before it executes any test.

@hubertp hubertp merged commit 048dea5 into develop Mar 5, 2025
72 of 73 checks passed
@hubertp hubertp deleted the wip/hubert/fix-deps-lib-manager branch March 5, 2025 21:42
@hubertp hubertp mentioned this pull request Mar 6, 2025
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CI: No changelog needed Do not require a changelog entry for this PR.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants