Add tc39/test262 tests in k6 #1747

mstoykov · 2020-11-30T14:48:36Z

https://github.com/tc39/test262/blob/main/INTERPRETING.md Should probably be used as reference ... about what the test metadata (and not only) means

codecov-io · 2020-11-30T14:56:57Z

Codecov Report

Merging #1747 (98c189f) into master (40979fd) will increase coverage by 0.06%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master    #1747      +/-   ##
==========================================
+ Coverage   71.40%   71.47%   +0.06%     
==========================================
  Files         178      178              
  Lines       13751    13777      +26     
==========================================
+ Hits         9819     9847      +28     
+ Misses       3320     3317       -3     
- Partials      612      613       +1

Flag	Coverage Δ
ubuntu	`71.42% <ø> (+0.06%)`	⬆️
windows	`70.02% <ø> (+0.06%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
ui/form.go	`58.33% <0.00%> (-5.31%)`	⬇️
js/runner.go	`80.45% <0.00%> (-0.66%)`	⬇️
js/modules/k6/http/request.go	`80.00% <0.00%> (-0.65%)`	⬇️
js/modules/k6/grpc/client.go	`78.82% <0.00%> (-0.36%)`	⬇️
cmd/common.go	`36.36% <0.00%> (ø)`
js/modules/k6/ws/ws.go	`81.56% <0.00%> (ø)`
js/modules/k6/html/html.go	`81.95% <0.00%> (ø)`
cmd/config.go	`84.92% <0.00%> (+1.58%)`	⬆️
lib/executor/vu_handle.go	`95.23% <0.00%> (+1.90%)`	⬆️
lib/types/types.go	`89.55% <0.00%> (+3.98%)`	⬆️
... and 1 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 40979fd...0e0e97e. Read the comment docs.

This reverts commit c5ab1f0.

imiric · 2020-12-02T09:29:34Z

.github/workflows/all.yml

@@ -83,6 +83,28 @@ jobs:
          fi
          go test "${args[@]}" -timeout 800s ./...

+  tc39:
+    runs-on: ubuntu-latest
+    steps:


Do we want to run this on every commit? We might be OK with just running them on each release (version tag), especially if they're flaky, and we should probably exclude it from the requirements for a successful build.

there are currently no requirements (I think)

they aren't really flaky ... just they sometimes take too much time - predomenantly when I increase them or decide to try to enable parallel execution ... OR maybe they take too much memory 🤷‍♂️

In the end we might move them to not being run on eveyr commit :D, but currently they are in development :P

We can move this to its own job, and have it run only when files in js were changed with https://docs.github.com/en/free-pro-team@latest/actions/reference/workflow-syntax-for-github-actions#onpushpull_requestpaths

That said, if it takes less than 10 minutes, this is probably good enough...

na--

I don't have time to dive into the nitty-gritty details (e.g. tc39TestCtx.runTC39Test() and how exactly the TC39 test suite is structured), but overall I quite like how you've structured the tests on the k6 side ❤️

@mstoykov, what do you think about exporting the ctx.errors to another JSON file, and saving it as a build artifact in github actions, not just printing it to stdout? That way it'd be easier to diff the changes when there are issues, and it shouldn't be too difficult to do.

na-- · 2020-12-09T12:08:50Z

.github/workflows/all.yml

+          cd TestTC39/test262
+          git init
+          git remote add origin https://github.com/tc39/test262.git
+          git fetch origin --depth=1 72154b17fc99a26e79b2586960f059360d4ce43d


What is so special about this commit? And if you don't want to use master because of potential future flakyness, can we have this commit ID as an environment variable, so it'd be a bit more visible and easier to update.

This was the latest commit at the time I started :D

na-- · 2020-12-09T12:11:45Z

.github/workflows/all.yml

@@ -83,6 +83,28 @@ jobs:
          fi
          go test "${args[@]}" -timeout 800s ./...

+  tc39:
+    runs-on: ubuntu-latest
+    steps:


We can move this to its own job, and have it run only when files in js were changed with https://docs.github.com/en/free-pro-team@latest/actions/reference/workflow-syntax-for-github-actions#onpushpull_requestpaths

That said, if it takes less than 10 minutes, this is probably good enough...

js/tc39/README.md

na-- · 2020-12-09T12:21:50Z

js/tc39/tc39_test.go

+}
+
+func TestTC39(t *testing.T) {
+	if testing.Short() {


I had no idea this existed... And I'm not sure I even like it, given the existence of build flags. but oh well... 🤷‍♂️ On the other hand, I ❤️ the skip message below, and how we can optionally include the tc39 tests in our normal golang tests, or not!

Co-authored-by: na-- <[email protected]>

mstoykov · 2020-12-10T10:12:46Z

@mstoykov, what do you think about exporting the ctx.errors to another JSON file, and saving it as a build artifact in github actions, not just printing it to stdout? That way it'd be easier to diff the changes when there are issues, and it shouldn't be too difficult to do.

My current way of doing it is to echo {} > breaking_test_errors.json and then get the json out of the output ... I would like to dump what the breaking_test_errors.json should be so the test pass ... but this will require some more monkeying with the ctx.errors as it currently only records what isn't in ctx.expectedErrors ... and I will need some way to merge them together - probably not hard, but I really prefer not to have to run the test 2-10 times again to check out that I did right.

I also do intend on coming back to enable/disable more tests and hopefully reorganize it so we can have proper concurrent execution ... and probably some memory usage reduction, so this will just be ... one more thing to fix ... next year :P

imiric

Fantastic work @mstoykov!!! 👏

I saw some commented out code in places, but there's probably a good reason you left it there, so it's fine.

I would echo Ned's suggestion to only run this if files in js change, or even on master commits and version tags only. We don't really need it for every PR, even if it takes less than 10 minutes.

js/tc39/checkout.sh

mstoykov · 2020-12-10T15:54:47Z

The workflows change seems to work #1764
It will run on every master push and tags and on PRs that changes anything in the js directory

imiric

Just squash/rebase.

PoC tc39/test262 tests in k6

1d2ac19

mstoykov requested review from imiric and na-- November 30, 2020 14:48

mstoykov added 8 commits November 30, 2020 18:39

Enable tc39/test262 sec-array

dd14b50

Add inline comments that tc39 code is based on dop251 work

f8a4b6d

refactor and appease the linter

6ab66b9

tc39/testdata make the directory structure match the name of the test

a74227a

tc39/test262: Enable some parallel execution

c5ab1f0

Revert "tc39/test262: Enable some parallel execution"

4039c37

This reverts commit c5ab1f0.

tc39/test262: Skip Intl and annexB/language

3d03c8b

Enable more tests

38d5fc0

imiric reviewed Dec 2, 2020

View reviewed changes

mstoykov added 9 commits December 2, 2020 12:19

tc39/test262: small optimization

0cd1e2f

half tests by just running in strict mode

7b15248

tc39/test262: Run only one of non/strict versions of each test

9c557fa

update breaking changes after last change

268565d

Block by path some tests

01a67f0

change breaking test errors based on previous commit 🤦

45d341e

Disable async/promise tests by path

7eb2ee8

Disable more tests through their path

e883572

don't run any async tests and update breaking changes

11aae1b

na-- previously approved these changes Dec 9, 2020

View reviewed changes

Update js/tc39/README.md

0de5490

Co-authored-by: na-- <[email protected]>

mstoykov dismissed na--’s stale review via 0de5490 December 9, 2020 15:25

Move the checkout in a script for easier maitnability

98c189f

imiric suggested changes Dec 10, 2020

View reviewed changes

imiric reviewed Dec 10, 2020

View reviewed changes

js/tc39/checkout.sh Show resolved Hide resolved

Add tc39 github workflow

bb1d4e1

mstoykov added 2 commits December 10, 2020 16:52

Add /js/tc39/TestTC39 to .gitignore

72a1bb8

Update js/tc39/Readme.md

91efee8

mstoykov changed the title ~~PoC tc39/test262 tests in k6~~ Add tc39/test262 tests in k6 Dec 10, 2020

mstoykov added 3 commits December 10, 2020 17:08

fix the tc39 workflow

0d03288

random not js change

0e0e97e

Run on every master or tag push

cf54873

mstoykov requested review from imiric and na-- December 10, 2020 15:56

imiric approved these changes Dec 10, 2020

View reviewed changes

mstoykov merged commit 15e15e5 into master Dec 14, 2020

mstoykov deleted the addTestTc39Test262 branch December 14, 2020 09:00

mstoykov added this to the v0.30.0 milestone Jan 8, 2021

This was referenced Jan 8, 2021

Run the tc39/test262 in travic CI dop251/goja#239

Merged

Run against the tc39 testsuite #1662

Closed

salem84 pushed a commit to salem84/k6 that referenced this pull request Feb 3, 2021

Add tc39/test262 tests in k6 (grafana#1747)

4993e39

na-- mentioned this pull request Apr 7, 2021

Update babel to 7.11.6. #1663

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add tc39/test262 tests in k6 #1747

Add tc39/test262 tests in k6 #1747

mstoykov commented Nov 30, 2020 •

edited

Loading

codecov-io commented Nov 30, 2020 •

edited

Loading

imiric Dec 2, 2020

mstoykov Dec 2, 2020

na-- Dec 9, 2020

na-- left a comment •

edited

Loading

na-- Dec 9, 2020

mstoykov Dec 9, 2020

na-- Dec 9, 2020

na-- Dec 9, 2020

mstoykov commented Dec 10, 2020

imiric left a comment

mstoykov commented Dec 10, 2020

imiric left a comment

Add tc39/test262 tests in k6 #1747

Add tc39/test262 tests in k6 #1747

Conversation

mstoykov commented Nov 30, 2020 • edited Loading

codecov-io commented Nov 30, 2020 • edited Loading

Codecov Report

imiric Dec 2, 2020

Choose a reason for hiding this comment

mstoykov Dec 2, 2020

Choose a reason for hiding this comment

na-- Dec 9, 2020

Choose a reason for hiding this comment

na-- left a comment • edited Loading

Choose a reason for hiding this comment

na-- Dec 9, 2020

Choose a reason for hiding this comment

mstoykov Dec 9, 2020

Choose a reason for hiding this comment

na-- Dec 9, 2020

Choose a reason for hiding this comment

na-- Dec 9, 2020

Choose a reason for hiding this comment

mstoykov commented Dec 10, 2020

imiric left a comment

Choose a reason for hiding this comment

mstoykov commented Dec 10, 2020

imiric left a comment

Choose a reason for hiding this comment

mstoykov commented Nov 30, 2020 •

edited

Loading

codecov-io commented Nov 30, 2020 •

edited

Loading

na-- left a comment •

edited

Loading