jsinterop-base icon indicating copy to clipboard operation
jsinterop-base copied to clipboard

Running tests?

Open niloc132 opened this issue 6 years ago • 6 comments

Hey guys, I'm looking at getting these tests running in open source, as I have a few changes I'd like to propose, and thought it might make sense to submit tests to help make my case. I nearly have the tests compiling, but one class is eluding me: com.google.common.annotations.UsedReflectively. I haven't found any trace of it on the internet - except for https://github.com/google/guava/issues/1617, which says (as of 2013) that it could get put somewhere more general for others to see it.

In context (and from the linked issue above), it might just be the equivalent of @SuppressWarnings("unused"), but considering how it is used, it could also have specific meaning to something like GWT->Closure or J2CL->Closure, in that the method name shouldn't be obfuscated. @JsMethod (with exports turned on during tests) would be enough to ensure that for GWT2 at least, but this annotation might have other work to do in cases involving closure.

Removing all trace of this annotation seems to at least get tests to compile (though I'm still missing a few stray dependencies). Is this something that can be made more general, so I could propose a patch to get tests running?

niloc132 avatar May 31 '18 03:05 niloc132

UsedReflectively is an internal annotation. I've created a change request internally in order to replace it by SuppressWarning

jDramaix avatar May 31 '18 04:05 jDramaix

Thanks Julien - I've got all dependencies (that I know of...) sorted, and the annotation removed, the next step is that I can't figure out how to tell bazel that I actually want sources of my dependencies. For example, the existing //third_party:gwt-jsinterop-annotations rule seems to point at the maven jar with both .class and .java, as well as .gwt.xml, but when run on that target, only classfiles are present, the sources have been stripped out. It seems like //third_party:libgwt-jsinterop-annotations-src.jar ought to point to the source jar (since maven_jar seems to build a jar for sources and a jar for bytecode, and a bazel java_import pointing at both), but the resulting jar actually is empty.

Is there a correct way in bazel to depend on a source jar from another target? The only alternative I'm currently seeing is to amend upstream gwt's user/BUILD.bazel to include the jsinterop targets, and depend on it that way.

https://github.com/bazelbuild/bazel/issues/308 apparently documents the addition of downloading distinct source jars, but this (and the resulting https://github.com/bazelbuild/bazel/issues/4798) shouldn't seem to apply, since the regular jar already had sources inside - we just need to get it on the classpath of the junit runner.

niloc132 avatar May 31 '18 04:05 niloc132

I didn't resolve the sources issue directly, but added a target to gwt's BUILD.bazel for the moment.

To get Truth working (which is shipped as a gwt-classified target), I had to replace the maven_jar impl with the skylark one. Frustratingly, this takes a completely different gav spec (order of artifact type and version is inverted in the string?!?), but lets you specify a different classifier, and then it seems to load correctly. Might work for sources too, will try that again so I can revert other changes.

A dozen or two new dependencies manually added later (will attempt the generate-workspace later to see if it gets it right on the first try from already-working poms), and I can run tests, but they are failing out of the box: JsTest and a few others have this code:

  // The extra indirection here prevents GWT optimization over params.
  @JsMethod(name = "getArguments", namespace = "jsinterop.base.GetArgumentsHelper")
  private static native JsArrayLike<Object> getArguments(Object... args);

This doesn't work as-is in GWT (though the comment seems to suggest that it should, and isn't J2CL specific? why else mention GWT by name?), but instead GetArgumentsHelper needs to be annotated as @JsType (and -generateJsInteropExports turned on of course), and then all tests pass. Is this a faulty comment, or export error in dropping the JsType a annotation, or perhaps is there another way that this should still work in GWT with just the JsMethod annotation?

I'm working to simplify my code now, but with all current tests passing I'm hoping to add a missing feature, and possibly correct a few minor oversights.

niloc132 avatar Jun 19 '18 23:06 niloc132

GetArgumentsHelper needs to be annotated as @JsType

Why is that?

gkdn avatar Jun 19 '18 23:06 gkdn

I'm not certain that this is the right answer, I'm asking, because it doesn't look right to me either way.

Without the JsType (and implied -generateJsInteropExports), the @JsMethod(name = "getArguments", namespace = "jsinterop.base.GetArgumentsHelper") needed because "The extra indirection here prevents GWT optimization over params." doesn't actually exist... doesn't it?

Test failure that I get here (one of seven that fail):

7) testArguments(jsinterop.base.JsTest)
com.google.gwt.core.client.JavaScriptException: (TypeError) : Cannot read property "base" from undefined
	at jsinterop.base.JsTest.testArguments(JsTest.java:212)
	at Unknown.anonymous(GWTTestMetadataImpl.java:62)
	at com.google.gwt.junit.client.impl.GWTTestAccessor.$invoke(GWTTestAccessor.java:35)
	at com.google.gwt.junit.client.impl.GWTRunner.$executeTestMethod(GWTRunner.java:226)
	at com.google.gwt.junit.client.GWTTestCase.$doRunTest(GWTTestCase.java:157)
	at com.google.gwt.junit.client.GWTTestCase.$runBare(TestCase.java:59)
	at com.google.gwt.junit.client.GWTTestCase.$__doRunTest(GWTTestCase.java:115)
	at com.google.gwt.junit.client.impl.GWTRunner.$runTest(GWTRunner.java:302)
	at com.google.gwt.junit.client.impl.GWTRunner.$doRunTest(GWTRunner.java:235)
	at com.google.gwt.junit.client.impl.GWTRunner$TestBlockListener.$onSuccess(GWTRunner.java:106)
	at com.google.gwt.junit.client.impl.GWTRunner$TestBlockListener.onSuccess(GWTRunner.java:100)
	at com.google.gwt.user.client.rpc.impl.RequestCallbackAdapter.$onResponseReceived(RequestCallbackAdapter.java:232)
	at com.google.gwt.http.client.Request.$fireOnResponseReceived(Request.java:250)
	at com.google.gwt.http.client.RequestBuilder$1.onReadyStateChange(RequestBuilder.java:412)
	at Unknown.anonymous(XMLHttpRequest.java:329)
	at com.google.gwt.core.client.impl.Impl.apply(Impl.java:309)
	at com.google.gwt.core.client.impl.Impl.entry0(Impl.java:368)
	at Unknown.anonymous(Impl.java:78)

The alternative that I had come up with so far was that this was supposed to have jsni and a //J2CL_ONLY, but that didn't feel right, given the "GWT" comment.

niloc132 avatar Jun 20 '18 01:06 niloc132

Doh, got it, I was half right at least. -generateJsInteropExports is indeed assumed, but the method in GetArgumentsHelper already has @JsMethod.

Okay, back to trying to generate deps automatically - only 1100 lines of generated bzl instead of 500 handwritten ones so far!

niloc132 avatar Jun 20 '18 01:06 niloc132