Skip to content

Fix setTextContainingScript Test #1466 #2175

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

amartya4256
Copy link
Contributor

@amartya4256 amartya4256 commented May 21, 2025

This PR fixes the test
org.eclipse.swt.tests.junit.Test_org_eclipse_swt_browser_Browser.test_setTextContainingScript_applicationLayerProgressListenerMustSeeUpToDateDom() as Edge seems to misbehave sporadically when an invalid script source is added using Edge:setText. Hence, with this commit we replace the script tag with a valid statement in the test and enable it.

Contributes to #1466

Copy link
Contributor

github-actions bot commented May 21, 2025

Test Results

   545 files  ±0     545 suites  ±0   28m 22s ⏱️ - 1m 0s
 4 398 tests ±0   4 380 ✅ ±0   18 💤 ±0  0 ❌ ±0 
16 719 runs  ±0  16 579 ✅ +1  140 💤  - 1  0 ❌ ±0 

Results for commit 1ade7ff. ± Comparison against base commit 9d3227d.

♻️ This comment has been updated with latest results.

@amartya4256 amartya4256 force-pushed the amartya4256/edge_browser_fix_test_setTextContainingScript_applicationLayerProgressListenerMustSeeUpToDateDom branch from 2309647 to 31184e7 Compare May 21, 2025 15:20
@amartya4256 amartya4256 marked this pull request as ready for review May 21, 2025 15:20
@amartya4256 amartya4256 force-pushed the amartya4256/edge_browser_fix_test_setTextContainingScript_applicationLayerProgressListenerMustSeeUpToDateDom branch from 31184e7 to 3dbeab4 Compare May 21, 2025 15:24
This commit fixes the test
org.eclipse.swt.tests.junit.Test_org_eclipse_swt_browser_Browser.test_setTextContainingScript_applicationLayerProgressListenerMustSeeUpToDateDom()
as Edge seems to misbehave sporadically when an invalid script source is
added using Edge:setText. Hence, with this commit we replace the
script tag with a valid statement in the test and enable it.

Contributes to eclipse-platform#1843
@amartya4256 amartya4256 force-pushed the amartya4256/edge_browser_fix_test_setTextContainingScript_applicationLayerProgressListenerMustSeeUpToDateDom branch from 3dbeab4 to 1ade7ff Compare May 23, 2025 10:05
Copy link
Contributor

@fedejeanne fedejeanne left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The change in the test looks fine.

Didn't we say we wanted to improve the JavaDoc of Browser::setText(String) to say mention that "we expect well-formed HTML (without broken scripts)" or something similar?

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

Successfully merging this pull request may close these issues.

Fix and reactivate browser tests executed on Edge
2 participants