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

Don't run EmptyDirTaskTests in a Docker container #3792

Merged
merged 6 commits into from
Jul 14, 2022

Conversation

dbwiddis
Copy link
Member

@dbwiddis dbwiddis commented Jul 6, 2022

Signed-off-by: Daniel Widdis widdis@gmail.com

Description

Docker doesn't provide the permissions to test EmptyDirTask. Similar to the Windows OS, the test should not run if it detects that environment.

The primary/reliable/consistent test of whether in a Docker container is the presence of a .dockerenv file in the root directory. There was mention in 2016 that this could in theory be removed in the future but it's still present 6 years later and other proposed alternatives are not consistently valid in 2022. I added two potential backup tests.

Issues Resolved

Fixes #3674

Check List

  • New functionality includes testing.
    • All tests pass
  • New functionality has been documented.
    • New functionality has javadoc added
  • Commits are signed per the DCO using --signoff

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

Signed-off-by: Daniel Widdis <widdis@gmail.com>
@dbwiddis dbwiddis requested review from a team and reta as code owners July 6, 2022 22:00
@github-actions
Copy link
Contributor

github-actions bot commented Jul 6, 2022

Gradle Check (Jenkins) Run Completed with:

Signed-off-by: Daniel Widdis <widdis@gmail.com>
Signed-off-by: Daniel Widdis <widdis@gmail.com>
@github-actions
Copy link
Contributor

github-actions bot commented Jul 7, 2022

Gradle Check (Jenkins) Run Completed with:

@github-actions
Copy link
Contributor

github-actions bot commented Jul 7, 2022

Gradle Check (Jenkins) Run Completed with:

Signed-off-by: Daniel Widdis <widdis@gmail.com>
@github-actions
Copy link
Contributor

github-actions bot commented Jul 8, 2022

Gradle Check (Jenkins) Run Completed with:

@owaiskazi19
Copy link
Member

@dbwiddis Precommit failed

/home/runner/work/OpenSearch/OpenSearch/buildSrc/src/test/java/org/opensearch/gradle/EmptyDirTaskTests.java:96: error: cannot find symbol
    private File getNewNonExistingTempFolderFile(Project project) throws IOException {
> Task :buildSrc:compileTestJava
                                                 ^
  symbol:   class Project
  location: class EmptyDirTaskTests
/home/runner/work/OpenSearch/OpenSearch/buildSrc/src/test/java/org/opensearch/gradle/EmptyDirTaskTests.java:49: error: cannot find symbol
        Project project = ProjectBuilder.builder().build();
        ^
  symbol:   class Project
  location: class EmptyDirTaskTests
/home/runner/work/OpenSearch/OpenSearch/buildSrc/src/test/java/org/opensearch/gradle/EmptyDirTaskTests.java:76: error: cannot find symbol
        Project project = ProjectBuilder.builder().build();
        ^
  symbol:   class Project
  location: class EmptyDirTaskTests
Note: Some input files use or override a deprecated API.
> Task :buildSrc:compileTestJava FAILED

Signed-off-by: Daniel Widdis <widdis@gmail.com>
Signed-off-by: Daniel Widdis <widdis@gmail.com>
@dbwiddis
Copy link
Member Author

dbwiddis commented Jul 8, 2022

@dbwiddis Precommit failed

The perils of editing on my home machine where I hadn't suppressed save actions and it went crazy. All fixed now.

@github-actions
Copy link
Contributor

github-actions bot commented Jul 8, 2022

Gradle Check (Jenkins) Run Completed with:

@github-actions
Copy link
Contributor

github-actions bot commented Jul 8, 2022

Gradle Check (Jenkins) Run Completed with:

@codecov-commenter
Copy link

Codecov Report

❗ No coverage uploaded for pull request base (main@289c2d3). Click here to learn what that means.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             main    #3792   +/-   ##
=======================================
  Coverage        ?   70.57%           
  Complexity      ?    56709           
=======================================
  Files           ?     4557           
  Lines           ?   272681           
  Branches        ?    40038           
=======================================
  Hits            ?   192456           
  Misses          ?    63971           
  Partials        ?    16254           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 289c2d3...8be1706. Read the comment docs.

Copy link
Member

@owaiskazi19 owaiskazi19 left a comment

Choose a reason for hiding this comment

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

LGTM! Thanks for the changes

@dbwiddis
Copy link
Member Author

Hi @reta, let me know if I need to make any changes here!

@reta
Copy link
Collaborator

reta commented Jul 14, 2022

Hi @reta, let me know if I need to make any changes here!

LGTM, I don't know better way to detect if JVM is running in Docker or not ...

@reta reta added the backport 2.x Backport to 2.x branch label Jul 14, 2022
@reta reta merged commit c526b0c into opensearch-project:main Jul 14, 2022
opensearch-trigger-bot bot pushed a commit that referenced this pull request Jul 14, 2022
* Don't run EmptyDirTaskTests in a Docker container

Signed-off-by: Daniel Widdis <widdis@gmail.com>

* Catch possible exceptions

Signed-off-by: Daniel Widdis <widdis@gmail.com>

* Fix newlines with spotless

Signed-off-by: Daniel Widdis <widdis@gmail.com>

* Add comments explaining test incompatibilities

Signed-off-by: Daniel Widdis <widdis@gmail.com>

* IDE incorrectly removed needed imports

Signed-off-by: Daniel Widdis <widdis@gmail.com>

* Manual edit missed a duplicate

Signed-off-by: Daniel Widdis <widdis@gmail.com>
(cherry picked from commit c526b0c)
@dbwiddis dbwiddis deleted the dockercheck branch July 15, 2022 17:59
owaiskazi19 pushed a commit that referenced this pull request Jul 15, 2022
* Don't run EmptyDirTaskTests in a Docker container

Signed-off-by: Daniel Widdis <widdis@gmail.com>

* Catch possible exceptions

Signed-off-by: Daniel Widdis <widdis@gmail.com>

* Fix newlines with spotless

Signed-off-by: Daniel Widdis <widdis@gmail.com>

* Add comments explaining test incompatibilities

Signed-off-by: Daniel Widdis <widdis@gmail.com>

* IDE incorrectly removed needed imports

Signed-off-by: Daniel Widdis <widdis@gmail.com>

* Manual edit missed a duplicate

Signed-off-by: Daniel Widdis <widdis@gmail.com>
(cherry picked from commit c526b0c)

Co-authored-by: Daniel Widdis <widdis@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport 2.x Backport to 2.x branch
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[BUG] testCreateEmptyDirNoPermissions when running gradle check in docker container
4 participants