Skip to content

waitForJobs: don't wait for setInterval() jobs.#115

Open
alexgo91 wants to merge 1 commit into
HtmlUnit:masterfrom
alexgo91:master
Open

waitForJobs: don't wait for setInterval() jobs.#115
alexgo91 wants to merge 1 commit into
HtmlUnit:masterfrom
alexgo91:master

Conversation

@alexgo91

@alexgo91 alexgo91 commented Jan 3, 2020

Copy link
Copy Markdown

In JavaScriptJobManagerImpl, method waitForJobs() waits for all kinds of
jobs, including never-ending jobs created by setInterval(). The caller
wouldn't expect this method to block indefinitely and it causes some
scenarios to get stuck.
Also, for correctness - synchronized block should wrap final job count
check as well.

In JavaScriptJobManagerImpl, method waitForJobs() waits for all kinds of
jobs, including never-ending jobs created by setInterval(). The caller
wouldn't expect this method to block indefinitely and it causes some
scenarios to get stuck.
Also, for correctness - synchronized block should wrap final job count
check as well.
@rbri

rbri commented Jan 4, 2020

Copy link
Copy Markdown
Member

What about using waitForJobsStartingBefore(final long delayMillis, final JavaScriptJobFilter filter)?

@alexgo91

alexgo91 commented Jan 6, 2020

Copy link
Copy Markdown
Author

I am thinking about the user of HtmlUnit - it will be more simpler for him to say "wait for all tasks", instead of defining filter + delay (which is not always clear as it depends on the performance of the website).
The wait command is mostly used as part of a larger test scenario, and therefore the user just wants the website to finish loading and become ready, including all tasks. It's easier to call waitForJobs - and there shouldn't be a scenario in which a user expects this call to block forever (like happens in case of setInterval() jobs)

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.

2 participants