Small patch, adding CharSequence::isEmpty

[vjay82]

Fixes #10091

[niloc132]

Thanks for the contribution - would you add a "quick" test for this? The test method itself should be quick (empty string, non-empty string, maybe mutate a StringBuilder to be empty, then non-empty, then empty again), but setting up the test is going to be a little irritating to add since the test won't compile with Java 11, it will need to have super-source.

• Start with a class like com.google.gwt.emultest.java15.lang.StringTest (can copy from the java11.lang.StringTest for copyright header, superclass), but it will need a special runTest method like Java17Test has to only run with a high enough source level.
• The test method itself needs to be a stub with no implementation, since it wouldn't compile otherwise.
• Then, make a copy of the class, with no runTest, and with the stub populated, in somewhere like user/test-super/com/google/gwt/dev/jjs/super/com/google/gwt/emultest/java15/lang/StringTest.java. This will be supersourced, so your IDE may not see it properly, and will correctly not be compiled in Java 11.

[vjay82]

Ok, will have a look when some free time is at hand,

[vjay82]

Hi,

I implement the tests as instructed.

Then I tried to let them run according to the instructions in the README.md.

Using Java 21:

set TZ=America/Los_Angeles
set ANT_OPTS=-Dfile.encoding=UTF-8
ant clean
ant user -Dtarget=test
...
[junit] Tests run: 5, Failures: 1, Errors: 0, Skipped: 0, Time elapsed: 0,721 sec
[junit] Test com.google.gwt.dev.javac.PersistentUnitCacheTest FAILED
...
[junit] Tests run: 24, Failures: 1, Errors: 0, Skipped: 0, Time elapsed: 19,753 sec
[junit] Test com.google.gwt.dev.jjs.impl.DeadCodeEliminationTest FAILED
...
BUILD FAILED

My new com.google.gwt.emultest.java15.lang.StringTest was never called. (Wouldn't CharSequenceTest be a better name?)

I checked where com.google.gwt.emultest.java11.lang.StringTest might be called/linked and found com.google.gwt.emultest.EmulJava11Suite.

I created a corresponding EmulJava15Suite but still no change.

I also checked if EmulJava11Suite is refered to anywhere, which is not the case.

The java11 StringTest seems also not to be listed (probably not called?) in the output = ?

I set my test up to fail:

com.google.gwt.emultest.java15.lang.StringTest extends EmulTestBase {
	
  public void testIsEmpty() {
    // empty stub
    assertFalse(true);
  }

}

(Also broke gwt\user\test-super\com\google\gwt\dev\jjs\super\com\google\gwt\dev\jjs\super\com\google\gwt\emultest\java15\lang\StringTest.java)

The output of the ant-call was the same.

I then tried to set up ...java11.lang.StringTest to fail.
Same output again.

Ich checked if DeadCodeEliminationTest (which is statet in the output) is referred to anywhere. It is not...

Finally, as I might have broken something, I reverted my changes back and ran the tests on the original repo:

set TZ=America/Los_Angeles
set ANT_OPTS=-Dfile.encoding=UTF-8
ant clean
ant user -Dtarget=test
...
[junit] Tests run: 5, Failures: 1, Errors: 0, Skipped: 0, Time elapsed: 0,68 sec
[junit] Test com.google.gwt.dev.javac.PersistentUnitCacheTest FAILED
...
[junit] Tests run: 24, Failures: 1, Errors: 0, Skipped: 0, Time elapsed: 18,227 sec
[junit] Test com.google.gwt.dev.jjs.impl.DeadCodeEliminationTest FAILED
...
BUILD FAILED

Exactly the same thing.

I tried to run "all" tests without "user" prefix - to make sure every single one should be run.
The command also fails, see all tests.txt

Any ideas?

[niloc132]

Would you push the commit with the draft test so I can take a look there, give it a try?

The way I run tests is usually based on the table, cd'ing into the directory and running a task or two, like this:

cd user/
ant test.web.htmlunit '-Dgwt.junit.testcase.web.includes=**/CharSequenceTest.class'

that will run every test with that name in user/ with htmlunit, with a production build (rather than draft, or disabling class metadata) - for a simple emulation test like this, that should be sufficient.

With your own github fork, you can also push a branch name with a -test suffix, and enable github actions for your fork - the entire test suite will run. Name your branch something like charsequence-isempty-test and push that to your fork.

---

I'm very far from an ant expert, and as far as I can tell that ant dev -Dtarget=test is nonsense - if you specify another task in dev but not other subprojects (like buildtools), it will fail right away when buildtools can't use it. It seems to be running the target task everywhere...rather than just in the specified project.

As to why those tests are failing for you locally, I'm not sure, do you want to open a separate issue for that? I would guess there is a windows specific issue with how the tests are defined.

[jnehlmeier]

DeadCodeEliminationTest is likely failing because of the Java default locale used on the system. There is a test that assumes US/en locale and it seems @vjay82 is located in Germany, like me. See: #10067

You have to run the tests using -Duser.country=US -Duser.language=en

[jnehlmeier]

Also if you are using macOS some tests will simply fail, because Java on macOS has a bad implementation of WatchService. See: #10069

[vjay82]

Hi.

@niloc132

With

cd user
ant test.web.htmlunit "-Dgwt.junit.testcase.web.includes=**/CharSequenceTest.class"

(it is " under Windows, otherwise straight into hell)

and a few other modifications I got it to run. Only flaw still is, that the override CharSequenceTest is never used (breaking the test there has no effect). I didn't use your folder structure proposal because I found another one quite similar but not identical. Maybe you can have a look.

@jnehlmeier
Thank you for the hint. I gave up on getting the general tests to run. Im too old for fighting with code that is written to only work on a specific os, locale or moon phase.

[niloc132]

and a few other modifications I got it to run. Only flaw still is, that the override CharSequenceTest is never used (breaking the test there has no effect). I didn't use your folder structure proposal because I found another one quite similar but not identical. Maybe you can have a look.

Thank you for keeping at this - there is a typo that prevents your test from working.

In the "regular" version of the CharSequenceTest, you placed it in package com.google.gwt.emultest.java.lang; However, your supersource version is in package com.google.gwt.emultest.java15.lang; - the difference in package (java vs java15) will mean that the supersource version will not be used when compiling to JS, so the test will never run.

[vjay82]

I had to add the stub under com.google.gwt.emultest.java.lang as well.
The reason was, that the test runner complained the test not being referenced in EmulSuite.java.
I tried to include it in there but it resulted in a compilation error that the class file could not be found.
I looked at the other tests and discovered their files also exiting under com.google.gwt.emultest.java.lang .
My conclusion was that these files probably get replaced when running under the appropriate Java version (however running your command line shows both test-files being executed.)

So you think moving the supersourced version to java.lang solves this?
Then we can skip the java15 stub, no?
(Wouldn't really matter as CharSequence supports the function independently anyway, right?)

[niloc132]

The stub in the test/ directory must exist since Java 11 can't compile this test, so instead Java 11 sees the stub.

With the addition of your supersource to the CharSequence type, the super-test/ code will be able to compile with references to Java 15 methods, even on Java 11.

The fully-qualified name of the class must match in both test/ and super-test/, and for EmulSuite to reference the class, it too must match the same name. For these purposes at least, the actual name doesn't matter, as long as all three locations are the same.

However, to keep track of "when can we un-supersource this test" and which tests belong where, we prefer tests to be named for the version of Java they are based on. So, the java15 package, or probably instead java17 (since Java 15 is already EOL) is the preferred package name to use - in all three places (for the stub in test/, the real test in test-super/, and the import in the suite itself).

[vjay82]

Sorry, I didn't get what needs to be done now.

[niloc132]

Your user/test-super/ implementation is in package java15. This is preferred to using the package java, which the stub in user/test/ is using. Move the stub to also use java15, and in turn, change the suite to also import the class from java15.

You've already updated the EmulSuite.gwt.xml to reference the java15 package as including sources, so that should be sufficient from what I can see here.

[vjay82]

Hm, I'm pretty sure that didn't work. Thats why I also copied the stub to java (without 15).
I will try again when time is available.

[niloc132]

As above, we'd really prefer java17 as the package (since we don't test with EOL JDKs). If you find it doesn't work with this package, please push what you have (to this or another branch) so we can help out, get this finished?

[vjay82]

I moved the tests to the requested java17 - paths and broke the super implementation.
Running the command still reports no errors.
I tried to figure out where to put the test-super class by a bit of trial and error but couldn't find a location where the command would fail.

So still the problem, that it is not executed.

[zbynek]

#10106 has the super-sourced tests for Java 17 emulation.
Maybe I can include your changes in it so that we don't have to worry about conflicts?

[vjay82]

@zbynek Yes, just go ahead 👍

[zbynek]

Now part of #10106