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

Remove references to jcenter.bintray.com which is being shutdown on May 1, 2021 #523

Merged
merged 1 commit into from
Feb 16, 2021

Conversation

cheister
Copy link
Collaborator

Fix for #517

@cheister cheister requested review from shs96c and jin February 16, 2021 07:16
@cheister cheister requested a review from c-parsons as a code owner February 16, 2021 07:16
@cheister cheister force-pushed the remove-jcenter-references branch 2 times, most recently from b1510ed to 2d90663 Compare February 16, 2021 07:44
@cheister
Copy link
Collaborator Author

cheister commented Feb 16, 2021

@jin Looks like //tests/com/jvm/external:UnsafeSharedCacheTest is failing on OSX. I'm not sure if #375 is now a problem for OSX and if //tests/com/jvm/external:UnsafeSharedCacheTest" should be commented out for OSX in .bazelci/presubmit.yml as well?

@jin
Copy link
Collaborator

jin commented Feb 16, 2021

The failure is

exec ${PAGER:-/usr/bin/less} "$0" || exit 1
Executing tests from //tests/com/jvm/external:UnsafeSharedCacheTest
-----------------------------------------------------------------------------
JUnit4 Test Runner
.E
Time: 0.031
There was 1 failure:
1) test_jarsOnClassPath_areInTheSharedCache(com.jvm.external.UnsafeSharedCacheTest)
java.lang.AssertionError: 
Expected: a string containing "/tmp/custom_coursier_cache"
     but: was "file:/private/var/tmp/_bazel_buildkite/0f1ff3967c5ac2afc60797312589b600/execroot/rules_jvm_external/bazel-out/darwin-fastbuild/bin/external/maven/v1/https/repo1.maven.org/maven2/org/hamcrest/hamcrest/2.1/processed_hamcrest-2.1.jar"
	at org.hamcrest.MatcherAssert.assertThat(MatcherAssert.java:18)
	at org.junit.Assert.assertThat(Assert.java:964)
	at org.junit.Assert.assertThat(Assert.java:930)
	at com.jvm.external.UnsafeSharedCacheTest.test_jarsOnClassPath_areInTheSharedCache(UnsafeSharedCacheTest.java:35)

FAILURES!!!
Tests run: 1,  Failures: 1


BazelTestRunner exiting with a return value of 1
JVM shutdown hooks (if any) will run now.
The JVM will exit once they complete.

-- JVM shutdown starting at 2021-02-16 07:45:59 --

which is different from the linked issue. I think this has something to do with a recent mac upgrade on CI. Looking.

@jin
Copy link
Collaborator

jin commented Feb 16, 2021

Could you try changing this line:

https://github.com/bazelbuild/rules_jvm_external/blob/367eb9ae5be8acd5a069b84c4672a2ed1d0bda82/.bazelci/presubmit.yml#L79

as well as UnsafeSharedCacheTest itself to look for /Users/buildkite/coursier_cache, instead of the /tmp dir? I'm guessing that something stateful happened in the CI upgrade that caused something odd with /tmp.

I tried changing it in #524 without changing the test, and it fails as expected.

@cheister cheister force-pushed the remove-jcenter-references branch from 2d90663 to ae0a163 Compare February 16, 2021 19:51
@cheister
Copy link
Collaborator Author

Ok, sorted it out the UnsafeSharedCacheTest was only working before because of a typo in the test looking for jcentray.bintray.com instead of jcenter.bintray.com, this filtered out the hamcrest dependencies. Since only the guava dependency is included in https://github.com/bazelbuild/rules_jvm_external/blob/master/tests/integration/BUILD#L66, that is the only one we should look for in the test with the coursier_cache path.

@cheister cheister force-pushed the remove-jcenter-references branch from ae0a163 to ff339ac Compare February 16, 2021 20:02
@cheister cheister force-pushed the remove-jcenter-references branch from ff339ac to 72c5de6 Compare February 16, 2021 20:06
@cheister cheister merged commit 82fca1f into bazel-contrib:master Feb 16, 2021
@cheister cheister deleted the remove-jcenter-references branch February 16, 2021 20:12
@jin
Copy link
Collaborator

jin commented Feb 16, 2021

Yikes, good catch.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants