Ensure all passing user tests are called by their appropriate suite

[niloc132]

Fixes #10258

[zbynek]

Listing all .*Test.java files, comparing with grep result for \w+Test.class in source and eliminating abstract classes + not actual tests gives this:

AbstractSequentialListTest.class - testclass itself not abstract
AtomicReferenceTest.class
BidiFormatterBaseTest.class
EmmaClassLoadingTest.class
HasAlignmentParserTest.class - no GWTTestcase
HorizontalAlignmentConstantParser_Test.class - no GWTTestcase
ImageElementTest.class
LayoutPanelTest.class
NoGenerateJsInteropExportsTest.class (@DoNotRunWith(Platform.Devel))
RPCTypeCheckArraysTest.class - no GWTTestcase
RPCTypeCheckCollectionsTest.class - no GWTTestcase
SafeHtmlBidiFormatterTest.class - no GWTTestcase
SelectionScriptJavaScriptTest.class - no GWTTestcase
StringReaderTest.class
VerticalAlignmentConstantParser_Test.class - no GWTTestcase

PR description specificaly mentions passing tests, I didn't try running any of these. The two test classes with underscore look like old versions of their conventionally named siblings and should be probably deleted.

[zbynek]

Please check if some of the classes in my previous comment could/should be enabled too.

[zbynek]

Three more:

com.google.gwt.dev.js.client.CoverageTest, com.google.gwt.validation.rebind.UtilTest, com.google.gwt.user.client.RandomTest

The last one is not really testing anything.

[niloc132]

BidiFormatterBaseTest is also not GWTTestCase - added to i18n jre suite
EmmaClassLoadingTest only makes sense when using emma and legacy dev mode - if it still works, that's great, but I don't want to sink time into fixing tests for it.

HorizontalAlignmentConstantParser_Test looks like an earlier iteration of HorizontalAlignmentConstantParserTest - added in the same commit, added now to the same suite. https://groups.google.com/g/google-web-toolkit-contributors/c/hYDKCdL1UMs/m/lzD9wdFuhF8J
RPCTypeCheckCollectionsTest fails, will create a new ticket for it.
SelectionScriptJavaScriptTest seems to be mentioned already in LinkerSuite

There's no suitable suite for CoverageTest, added one.

[niloc132]

NoGenerateJsInteropExportsTest isn't safe to add to the usual test runs, since -generateJsInteropExports is always specified, we'll need a custom run that handles this, will split to a new issue.

#10301

[niloc132]

StringReaderTest has three failures:

gwt/user/test/com/google/gwt/emultest/java/io/StringReaderTest.java

Lines 41 to 46 in 378fd96

	StringReader reader = new StringReader("The q\u00DCuick brown fox jumped over the lazy dog");
	assertEquals('T', reader.read());
	char[] buf = new char[10];
	assertEquals(6, reader.read(buf, 3, 6));
	assertEquals("\u0000\u0000\u0000he q\u00DCi\u0000", String.valueOf(buf));

junit.framework.AssertionFailedError: expected: <   he qÜi >, actual: <   he qÜu >

I think that assertion is just wrong on 46, should be a u instead of a i - or, more likely, the sample data should have dropped the u

gwt/user/test/com/google/gwt/emultest/java/io/StringReaderTest.java

Lines 78 to 81 in 378fd96

	public void testSkip_stringOverBufferSize() throws Exception {
	StringReader reader = new StringReader(repeat('x', 1025));
	assertEquals(1025, reader.skip(2000));

java.lang.StringIndexOutOfBoundsException: fromIndex: 0, toIndex: 1025, length: 1024
	at java.lang.Throwable.Throwable(Throwable.java:66)
	at java.lang.Exception.Exception(Exception.java:29)
	at java.lang.RuntimeException.RuntimeException(RuntimeException.java:29)
	at java.lang.IndexOutOfBoundsException.IndexOutOfBoundsException(IndexOutOfBoundsException.java:29)
	at java.lang.StringIndexOutOfBoundsException.StringIndexOutOfBoundsException(StringIndexOutOfBoundsException.java:30)
	at javaemul.internal.InternalPreconditions.checkCriticalStringBounds(InternalPreconditions.java:585)
	at java.lang.String.$getChars(String.java:452)
	at java.lang.String.getChars_II_CI_V__devirtual$(String.java:450)
	at java.io.StringReader.read(StringReader.java:47)
	at java.io.Reader.skip(Reader.java:94)
	at com.google.gwt.emultest.java.io.StringReaderTest.testSkip_stringOverBufferSize(StringReaderTest.java:81)

This could be an implementation bug in the reader, or possibly something changed in String that broke this. Given the above, I suspect StringReader to be at fault. The same test passes on the JVM.

gwt/user/test/com/google/gwt/emultest/java/io/StringReaderTest.java

Lines 86 to 94 in 378fd96

	public void testSkip_longString() throws Exception {
	int n = 10000;
	char[] arr = new char[n];
	for (char i = 0; i < n; i++) {
	arr[i] = i;
	}
	StringReader reader = new StringReader(String.valueOf(arr));
	assertEquals(5000, reader.skip(5000));

java.lang.StringIndexOutOfBoundsException: fromIndex: 0, toIndex: 5000, length: 1024
	at java.lang.Throwable.Throwable(Throwable.java:66)
	at java.lang.Exception.Exception(Exception.java:29)
	at java.lang.RuntimeException.RuntimeException(RuntimeException.java:29)
	at java.lang.IndexOutOfBoundsException.IndexOutOfBoundsException(IndexOutOfBoundsException.java:29)
	at java.lang.StringIndexOutOfBoundsException.StringIndexOutOfBoundsException(StringIndexOutOfBoundsException.java:30)
	at javaemul.internal.InternalPreconditions.checkCriticalStringBounds(InternalPreconditions.java:585)
	at java.lang.String.$getChars(String.java:452)
	at java.lang.String.getChars_II_CI_V__devirtual$(String.java:450)
	at java.io.StringReader.read(StringReader.java:47)
	at java.io.Reader.skip(Reader.java:94)
	at com.google.gwt.emultest.java.io.StringReaderTest.testSkip_longString(StringReaderTest.java:94)

Looks like the same kind of failure as above.

#10300

[niloc132]

LayoutPanelTest fails with a timeout (at least in HtmlUnit)

gwt/user/test/com/google/gwt/user/client/ui/LayoutPanelTest.java

Lines 38 to 43 in 378fd96

	/**
	* Tests for a bug in LayoutCommand, which caused an animate() call, just
	* before an unnecessary forceLayout(), to get stuck. See issue 4360.
	*/
	@SuppressWarnings("deprecation")
	public void testRedundantForceLayout() {

#4360 is where this test came from, and the test was disabled in c608a72. No commit message, just a dead link to the old review system. At one point it was also marked as not running with HtmlUnit, but it is hard to be sure if that's still the case.

#10299

[niloc132]

CoverageTest isn't needed to be called by a suite after all, it has a test run just for it, and fails without the extra setup:

gwt/user/build.xml

Lines 539 to 555 in 378fd96

	<target name="test.coverage.htmlunit"
	depends="compile, compile.tests"
	description="Run tests for coverage support"
	unless="test.coverage.htmlunit.disable">
	<fileset id="test.coverage.htmlunit.tests" dir="${javac.junit.out}"
	includes="com/google/gwt/dev/js/client/CoverageTest.class"
	excludes=""/>
	<gwt.junit test.name="test.coverage.htmlunit"
	test.args="${test.args} -draftCompile -out www -prod"
	test.jvmargs="${test.jvmargs} -Dgwt.coverage=com/google/gwt/dev/js/client/CoverageTestModule.java,"
	test.out="${junit.out}/coverage-htmlunit"
	test.cases="test.coverage.htmlunit.tests">
	<extraclasspaths>
	<path refid="test.extraclasspath"/>
	</extraclasspaths>
	</gwt.junit>
	</target>

[zbynek]

Thanks for checking all of them.

• What about AbstractSequentialListTest?
• The reason SelectionScriptJavaScriptTest showed up in my check was actually the opposite of what this PR is doing -- this test is in a user suite, but probably shouldn't be (the test itself is in dev rather than user) -- that might be an issue for Mavenization too (?)

[zbynek]

(just the two questions from my previous comment)

[niloc132]

AbstractSequentialListTest should be included, yes.

I'm not worried about SelectionScriptJavaScriptTest being in the wrong source dir - it isn't the only one, but i will make sure it ends up where it belongs. The whole mavenization script is an exercise in finding special cases (or giving up and acknowledging that the entire use case is too special to be fixed, e.g. "all of the gwt module cycles"). My hunch is that this test actually runs twice - once in the dev tests (which, broadly, don't use suites anyway), and again in the user tests when referenced by the suite.