Skip to content

A simpler, smaller and faster implementation of String.replace - #10193

Closed
vjay82 wants to merge 2 commits into
gwtproject:mainfrom
vjay82:main
Closed

vjay82 wants to merge 2 commits into
gwtproject:mainfrom
vjay82:main

Conversation

@vjay82

@vjay82 vjay82 commented Nov 20, 2025

Copy link
Copy Markdown

As discussed in #10178

@niloc132 niloc132 added the ready This PR has been reviewed by a maintainer and is ready for a CI run. label Nov 21, 2025
@niloc132

Copy link
Copy Markdown
Member

Something funny is happening in the tests for this change, I'm suspicious that this actually changes behavior in some fundamental way:

...
    [junit] Tests run: 128, Failures: 0, Errors: 113, Skipped: 0, Time elapsed: 1,240.88 sec
    [junit] Test com.google.gwt.core.CoreSuite FAILED
    [junit] Running com.google.gwt.core.ext.LinkerSuite
    [junit] Tests run: 29, Failures: 0, Errors: 10, Skipped: 0, Time elapsed: 1,223.858 sec
    [junit] Test com.google.gwt.core.ext.LinkerSuite FAILED
    [junit] Running com.google.gwt.debug.DebugSuite
    [junit] Tests run: 2, Failures: 0, Errors: 1, Skipped: 0, Time elapsed: 1,221.307 sec
    [junit] Test com.google.gwt.debug.DebugSuite FAILED
...

These suites don't usually take 20+ minutes to run. It doesn't seem to be a fluke, I re-ran them and they still timed out the build.

I'm not sure why this doesn't impact other runs - we're triggering the timeout for "this test isn't doing anything, kill it"

The browser did not complete the test method com.google.gwt.core.Core.JUnit:com.google.gwt.core.client.GWTTest.testCreate in 1200000ms.
  We have no results from:
127.0.0.1 / Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:94.0) Gecko/20100101 Firefox/94.0
Actual time elapsed: 1200.146 seconds.
Try increasing this timeout using the '-testMethodTimeout minutes' option

com.google.gwt.junit.client.TimeoutException: The browser did not complete the test method com.google.gwt.core.Core.JUnit:com.google.gwt.core.client.GWTTest.testCreate in 1200000ms.
  We have no results from:
127.0.0.1 / Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:94.0) Gecko/20100101 Firefox/94.0
Actual time elapsed: 1200.146 seconds.
Try increasing this timeout using the '-testMethodTimeout minutes' option

There is an error from htmlunit, it looks like our htmlunit build is too old to reliably use replaceAll:

TypeError: Cannot find function replaceAll in object com.google.gwt.core.Core.JUnit/junit.html. (script in about:blank from (-1, -1) to (-1, -1)#1)

replaceAll is relatively new, standard as of 2020 https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/String/replaceAll#browser_compatibility - our release of htmlunit is missing this, so we might need to back-burner this until we're ready for an htmlunit version bump (like #10113). In theory, we could do that while staying on ant with #10115, or wait until we're away from ant, and then just bump to a new release.

@niloc132 niloc132 added this to the 2.14 milestone Nov 21, 2025
@zbynek

zbynek commented Nov 21, 2025

Copy link
Copy Markdown
Collaborator

Minimal version of HtmlUnit for this is 3.11.0 HtmlUnit/htmlunit-rhino-fork@3f5f6cc

@vjay82 vjay82 closed this by deleting the head repository Mar 31, 2026
@niloc132

niloc132 commented Apr 1, 2026

Copy link
Copy Markdown
Member

@vjay82 would it be okay with you if we still used this code once we're able to update?

@vjay82

vjay82 commented Apr 8, 2026

Copy link
Copy Markdown
Author

Yes. I just did not know the PR getting closed when removing the repository.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready This PR has been reviewed by a maintainer and is ready for a CI run.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants