timeout are not working on windows. #4378

lygstate · 2020-12-28T17:33:48Z

Fix the test262 timeout on Windows

Use the platform independent python threading.Timer instead of the unix only timeout tool.

The timeout error are like the following things:

C:\Users\lygstate>timeout -version
Error: The specified timeout (/T) value is invalid. The valid range is from -1 to 99999 seconds.

C:\Users\lygstate>timeout 0 python
Error: invalid syntax. The default option does not allow more than '1' times.
Type "TIMEOUT /?" to learn how to use it.

timeout option are added for future ES5.1 test262 usage
in pull request
#4391

The timeout setting to 60 to getting maximal tolerance
By doing this also fixes tests for es2015:
built-ins/decodeURIComponent/S15.1.3.2_A2.5_T1.js
built-ins/decodeURI/S15.1.3.1_A2.5_T1.js

JerryScript-DCO-1.0-Signed-off-by: Yonggang Luo luoyonggang@gmail.com

akosthekiss

Please, change commit message. The language used throughout the project is English.

Please, fix pylint check errors.

tools/runners/test262-harness.py

ossy-szeged · 2020-12-30T10:37:50Z

The change looks good, but I would change the commit log to be more descriptive. Now it says what was wrong, but it should say what have been done in the commit. For example:

Fix the test262 timeout on Windows

Use the platform independent python threading.Timer instead of the unix only timeout tool.

lygstate · 2020-12-30T10:59:37Z

The change looks good, but I would change the commit log to be more descriptive. Now it says what was wrong, but it should say what have been done in the commit. For example:

Fix the test262 timeout on Windows

Use the platform independent python threading.Timer instead of the unix only timeout tool.

done

lygstate · 2020-12-31T05:52:48Z

Please, change commit message. The language used throughout the project is English.

Please, fix pylint check errors.

Fixed

lygstate · 2020-12-31T05:53:49Z

tools/runners/test262-harness.py

        })
-        (code, out, err) = TestCase.execute(command)
+        # Only es2015 or esnext need the timeout option.
+        (code, out, err) = TestCase.execute(command, 60)


Add timeout option for the usage in es5.1,
setting to 60 for pass two time consuming tests.

I'm not sure if it is a good idea to have 60 seconds timeout unconditionally,
because language/statements/for-of/iterator-next-reference.js in ES2015-test262
is a wrong test (outdated because of spec change) and stucks in an infinite loop.

Increasing timeout means that running ES2015-test262 will take an extra minute because of this wrong test.
8 seconds timeout is enough to remove 2 slow tests from es2015 excludelist. And tests run in 80 seconds
for me with 8 seconds timeout, but in 80+57 seconds with 60 seconds timeout.

In nutshell: I would keep general timeout as low as possible, but we could add extra timeout for ES5.1 if it is necessary.

60 is needed to pass two new tests

I'm not sure if it is a good idea to have 60 seconds timeout unconditionally,
because language/statements/for-of/iterator-next-reference.js in ES2015-test262
is a wrong test (outdated because of spec change) and stucks in an infinite loop.

Increasing timeout means that running ES2015-test262 will take an extra minute because of this wrong test.
8 seconds timeout is enough to remove 2 slow tests from es2015 excludelist. And tests run in 80 seconds
for me with 8 seconds timeout, but in 80+57 seconds with 60 seconds timeout.

In nutshell: I would keep general timeout as low as possible, but we could add extra timeout for ES5.1 if it is necessary.

60s needed to pass

These two tests consume lot of time in jerryscript, maybe it's a performance issue.

The bottleneck is the wrong language/statements/for-of/iterator-next-reference.js in test262-es2015. If we increase the timeout, we unnecessarily increase the runtime, because we have to wait to kill this wrong test. Otherwise the 2 unskipped es2015 test is also covered by esnext, because test262 maintainers know these are slow tests and refactored them to be faster. So we don't loose anything if we keep the timeout as is and leave these tests skipped.

The bottleneck is the wrong language/statements/for-of/iterator-next-reference.js in test262-es2015. If we increase the timeout, we unnecessarily increase the runtime, because we have to wait to kill this wrong test. Otherwise the 2 unskipped es2015 test is also covered by esnext, because test262 maintainers know these are slow tests and refactored them to be faster. So we don't loose anything if we keep the timeout as is and leave these tests skipped.

Not that simple, the debug version build jerryscript are slower, we need more time tolerance, and that won't increate the total test262 running time much, because tests are running in parallel, the testes consume lot of time won't be a bottleneck.

@ossy-szeged Revert to 5 seconds.

LaszloLango

LGTM

tools/runners/test262-harness.py

lygstate · 2021-01-15T17:34:39Z

@akosthekiss timeout parameter removed, please take a look

tools/runners/test262-harness.py

Use the platform independent python threading.Timer instead of the unix only timeout tool. The timeout error are like the following things: ``` C:\Users\lygstate>timeout -version Error: The specified timeout (/T) value is invalid. The valid range is from -1 to 99999 seconds. C:\Users\lygstate>timeout 0 python Error: invalid syntax. The default option does not allow more than '1' times. Type "TIMEOUT /?" to learn how to use it. ``` JerryScript-DCO-1.0-Signed-off-by: Yonggang Luo luoyonggang@gmail.com

akosthekiss

LGTM

ossy-szeged

LGTM

akosthekiss requested changes Dec 28, 2020

View reviewed changes

lygstate force-pushed the timeout-windows branch 6 times, most recently from f6de887 to 26fef50 Compare December 29, 2020 04:24

ossy-szeged reviewed Dec 29, 2020

View reviewed changes

tools/runners/test262-harness.py Show resolved Hide resolved

tools/runners/test262-harness.py Outdated Show resolved Hide resolved

lygstate force-pushed the timeout-windows branch 2 times, most recently from 34876a0 to 2ad8357 Compare December 29, 2020 14:15

lygstate requested review from akosthekiss and ossy-szeged December 30, 2020 09:50

lygstate force-pushed the timeout-windows branch from 2ad8357 to 947ec64 Compare December 30, 2020 10:54

lygstate force-pushed the timeout-windows branch 6 times, most recently from c08e984 to 1f0c7e1 Compare December 30, 2020 15:46

lygstate mentioned this pull request Dec 30, 2020

Using a single test262-harness.py for es5.1, es2015 and esnext. #4389

Closed

lygstate commented Dec 31, 2020

View reviewed changes

lygstate force-pushed the timeout-windows branch from 1f0c7e1 to f615a7c Compare January 6, 2021 06:38

LaszloLango approved these changes Jan 11, 2021

View reviewed changes

akosthekiss reviewed Jan 11, 2021

View reviewed changes

tools/runners/test262-harness.py Outdated Show resolved Hide resolved

lygstate force-pushed the timeout-windows branch 3 times, most recently from 0083443 to d4fd6f5 Compare January 13, 2021 03:59

lygstate force-pushed the timeout-windows branch 2 times, most recently from 9c532cb to d4fd6f5 Compare January 13, 2021 04:02

lygstate requested a review from akosthekiss January 15, 2021 04:10

lygstate force-pushed the timeout-windows branch 3 times, most recently from 385549a to a460fd0 Compare January 15, 2021 17:16

akosthekiss requested changes Jan 15, 2021

View reviewed changes

tools/runners/test262-harness.py Outdated Show resolved Hide resolved

lygstate force-pushed the timeout-windows branch from a460fd0 to d8622de Compare January 15, 2021 19:53

lygstate requested a review from akosthekiss January 15, 2021 20:00

akosthekiss approved these changes Jan 16, 2021

View reviewed changes

akosthekiss added tools Related to the tooling scripts windows Windows specific labels Jan 16, 2021

ossy-szeged approved these changes Jan 16, 2021

View reviewed changes

ossy-szeged merged commit 2bad20a into jerryscript-project:master Jan 16, 2021

lygstate deleted the timeout-windows branch January 17, 2021 05:22

timeout are not working on windows. #4378

timeout are not working on windows. #4378

Uh oh!

Conversation

lygstate commented Dec 28, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

akosthekiss left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

ossy-szeged commented Dec 30, 2020

Uh oh!

lygstate commented Dec 30, 2020

Uh oh!

lygstate commented Dec 31, 2020

Uh oh!

lygstate Dec 31, 2020

Choose a reason for hiding this comment

Uh oh!

ossy-szeged Jan 11, 2021

Choose a reason for hiding this comment

Uh oh!

lygstate Jan 11, 2021

Choose a reason for hiding this comment

Uh oh!

lygstate Jan 11, 2021

Choose a reason for hiding this comment

Uh oh!

ossy-szeged Jan 13, 2021

Choose a reason for hiding this comment

Uh oh!

lygstate Jan 13, 2021

Choose a reason for hiding this comment

Uh oh!

lygstate Jan 15, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

LaszloLango left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

lygstate commented Jan 15, 2021

Uh oh!

Uh oh!

akosthekiss left a comment

Choose a reason for hiding this comment

Uh oh!

ossy-szeged left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

lygstate commented Dec 28, 2020 •

edited

Loading

lygstate Jan 15, 2021 •

edited

Loading