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

Use JUnit TemporaryFolder rule instead of system temp folder #6070

Merged
merged 2 commits into from Aug 16, 2018

Conversation

yuppie-flu
Copy link
Contributor

This should help to avoid transient failures, mentioned in #6013 (comment) when running tests in parallel. Otherwise several tests use the same /tmp working folder.

Copy link
Contributor

@gianm gianm left a comment

Choose a reason for hiding this comment

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

LGTM after CI 👍

@gianm
Copy link
Contributor

gianm commented Jul 30, 2018

Hi @yuppie-flu, there seems to be some problem with the new forbidden-api check:

[ERROR] Failed to execute goal de.thetaphi:forbiddenapis:2.3:check (validate) on project java-util: Parsing signatures failed: Class 'org.apache.commons.io.FileUtils' not found on classpath while parsing signature: org.apache.commons.io.FileUtils#getTempDirectory() -> [Help 1]

@yuppie-flu yuppie-flu force-pushed the temporary-folder branch 2 times, most recently from e2069d6 to 20160f1 Compare July 30, 2018 18:23
@yuppie-flu
Copy link
Contributor Author

Thank you @gianm
I had to remove this check, cause there are some modules, which do not depend on apache commons-io. forbiddenapis maven plugin is configured only once in parent pom.xml.
If we really need to forbid this, the simplest solution would be overriding failOnMissingClasses property I guess.

@leventov
Copy link
Member

@yuppie-flu failOnUnresolvableSignatures. Please set it to true and include FileUtils in the forbidden-apis config, it's fine.

Also in the future, don't force push into the PR branch, only add commits on top. See https://github.com/apache/incubator-druid/blob/master/CONTRIBUTING.md

@yuppie-flu
Copy link
Contributor Author

@leventov Done, thank you

@jihoonson jihoonson merged commit 62e5800 into apache:master Aug 16, 2018
@yuppie-flu yuppie-flu deleted the temporary-folder branch August 23, 2018 09:08
@dclim dclim added this to the 0.13.0 milestone Oct 8, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants