CI workflow improvements (mostly Test262) #889

Razican · 2020-10-18T19:22:23Z

This requires some testing and probably some improvements/fixes, but I wanted to open a PR to get some feedback.

I have moved the PR workflows to run on pull_request_target instead of pull_request. This should allow the criterion comparisons to be posted as comments even for people outside the org, I think.

I have also added a system to compare two Test262 results. For now, the comparison is very simple, it will just print the counts of passed/ignored/failed/panics in Test262 results. But it will generate some Markdown and it should post a comment in the PR. This comment would be re-written on each change (so no new comments would be added).

I have also improved a bit how the Test262 testing is done. We now ignore some extra tests that could not be passed. We now count as 2 the tests that need to be executed twice (in strict mode and in non-strict mode). And, I added an extra parameter to the parser to directly start in strict mode, so that we can test that.

There are some things left in the tester that is probably making some tests fail:

We are not creating the $262 object yet (some of its methods will be tricky to implement)
The print() global function just panics as unimplemented for now (but we are ignoring await tests, so it's not a big deal)
We don't follow 100% the negative results, since we are not really working with parsing/early/resolution/runtime errors, but should work for some cases.

This requires a small change in the results format, that I should have included in my last PR, but I forgot, so I might need to do an extra PR to fix results in the GitHub Pages.

I also took the opportunity to update the test suite and to upgrade all dependencies.

codecov · 2020-10-18T19:31:43Z

Codecov Report

Merging #889 into master will decrease coverage by 0.35%.
The diff coverage is 6.15%.

@@            Coverage Diff             @@
##           master     #889      +/-   ##
==========================================
- Coverage   59.79%   59.44%   -0.36%     
==========================================
  Files         165      166       +1     
  Lines       10410    10474      +64     
==========================================
+ Hits         6225     6226       +1     
- Misses       4185     4248      +63

Impacted Files	Coverage Δ
boa/src/value/mod.rs	`74.58% <ø> (ø)`
boa_cli/src/main.rs	`33.33% <ø> (ø)`
boa_tester/src/exec.rs	`0.00% <0.00%> (ø)`
boa_tester/src/main.rs	`0.00% <0.00%> (ø)`
boa_tester/src/read.rs	`0.00% <0.00%> (ø)`
boa_tester/src/results.rs	`0.00% <0.00%> (ø)`
boa_wasm/src/lib.rs	`0.00% <0.00%> (ø)`
boa/src/context.rs	`61.64% <100.00%> (ø)`
boa/src/lib.rs	`86.36% <100.00%> (ø)`
boa/src/syntax/parser/mod.rs	`53.84% <100.00%> (+1.84%)`	⬆️
... 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 4805e39...7c5e9b7. Read the comment docs.

Razican · 2020-10-19T11:13:53Z

I think this is now ready for review. I would like some feedback on this:

Removed some test that now pass properly from the ignore list. This
now also allows filtering out features, so I filtered out the
`TypedArray` feature that was failing all the tests. There are
other 3 features (at least) that we don't implement, but they seem
to have passing tests, so I commented them out.

And any general feedback about the PR.

This improves several things in the CI workflows: - More conformant Test262 result generation - Benchmarks should now show comments for all users - Added Test262 result comparison comments to Pull Requests

boa_tester/src/main.rs

HalidOdat

This looks awesome! :D

BTW shouldn't the test262 comment be posted on this PR

Razican · 2020-10-19T12:48:24Z

This looks awesome! :D

BTW shouldn't the test262 comment be posted on this PR

Good question, but since I have changed the pull_request triggers to pull_request_target, it will run the flows as if it were in the master branch. This allows adding comments by anyone (even non-project members) as far as I understand, but will only work if the trigger is in master.

Something I could try would be to temporarily set the workflow trigger to pull_request to see if it works. But for it to work, we need to first merge #895. But, in that case, we could not merge any other PR before this one (or we would break the CI).

As you can see, we are not running neither the benchmarks nor the Test262 suite in this PR.

HalidOdat · 2020-10-19T12:56:15Z

Something I could try would be to temporarily set the workflow trigger to pull_request to see if it works. But for it to work, we need to first merge #895. But, in that case, we could not merge any other PR before this one (or we would break the CI).

we could try this, there aren't any PRs (besides #887 but review is still needed) that are ready to be merged anyway, even if there were we can postpone them.

Razican · 2020-10-19T13:27:29Z

Something I could try would be to temporarily set the workflow trigger to pull_request to see if it works. But for it to work, we need to first merge #895. But, in that case, we could not merge any other PR before this one (or we would break the CI).

we could try this, there aren't any PRs (besides #887 but review is still needed) that are ready to be merged anyway, even if there were we can postpone them.

OK, let's merge it and I will change the triggers. Here are the new results, though:

Results:
Total tests: 78413
Passed tests: 18185
Ignored tests: 15507
Failed tests: 44721 (panics: 13519)
Conformance: 23.19%

Conformance is a bit higher, and we now properly count tests that we run twice.

HalidOdat · 2020-10-19T14:24:18Z

@Razican the workflow seems to be failing on Write a new comment with error Error: Missing comment 'body'.

github-actions · 2020-10-19T15:02:53Z

Test262 conformance changes:

Test result	master count	PR count	difference
Total	38,976	78,413	+39,437
Passed	8,635	18,189	+9,554
Ignored	5,577	15,507	+9,930
Failed	24,764	44,717	+19,953
Panics	0	13,519	+13,519
Conformance	22.15	23.20	+1.04%

Razican · 2020-10-19T15:06:06Z

It seems it's not capturing the full comment markdown, I will re-check that.

HalidOdat · 2020-10-19T15:35:57Z

It seems it's not capturing the full comment markdown, I will re-check that.

also shouldn't it be overriding the original comment?

.github/workflows/pull_request.yml

Razican · 2020-10-19T18:38:42Z

It seems it's not capturing the full comment markdown, I will re-check that.

also shouldn't it be overriding the original comment?

Yep, I have identified 2 bugs: it ignores lines after the first one in the output of the comparison, and it will not override the previous comment. I will give them a look now.

Razican · 2020-10-19T19:13:40Z

I think this is ready to be merged. It looks that the comments and so on are working. I don't think we can check if the pull_request_target works until we actually get a Pull Request (or somebody synchronizes theirs).

Razican added enhancement New feature or request test Issues and PRs related to the tests. labels Oct 18, 2020

Razican added this to the v0.11.0 milestone Oct 18, 2020

Razican marked this pull request as ready for review October 19, 2020 11:14

Razican requested review from Lan2u, HalidOdat, jasonwilliams and RageKnify October 19, 2020 11:14

Improved CI workflows

118fcf8

This improves several things in the CI workflows: - More conformant Test262 result generation - Benchmarks should now show comments for all users - Added Test262 result comparison comments to Pull Requests

Razican force-pushed the test262_wf_impr branch from 9fac5a6 to 118fcf8 Compare October 19, 2020 11:29

RageKnify reviewed Oct 19, 2020

View reviewed changes

boa_tester/src/main.rs Outdated Show resolved Hide resolved

Razican mentioned this pull request Oct 19, 2020

New result format #895

Merged

Fixed typo

d216a16

HalidOdat approved these changes Oct 19, 2020

View reviewed changes

Razican added 2 commits October 19, 2020 15:28

Checking the comment generation

0333a9a

Fixing conditions to test comments

14b228a

RageKnify approved these changes Oct 19, 2020

View reviewed changes

Fix a couple of bugs on the comparator

106d720

Fixed format

c4a2c41

jasonwilliams reviewed Oct 19, 2020

View reviewed changes

.github/workflows/pull_request.yml Show resolved Hide resolved

jasonwilliams approved these changes Oct 19, 2020

View reviewed changes

boa-dev deleted a comment from github-actions bot Oct 19, 2020

Trying to fix comment updating

fbdbe88

boa-dev deleted a comment from github-actions bot Oct 19, 2020

Razican added 3 commits October 19, 2020 20:57

Removing commit autor when searching

4b91af9

Replace the comment instead of appending

17ea21a

Switched back to the pull_request_target event

7c5e9b7

jasonwilliams merged commit be5d198 into master Oct 19, 2020

Razican deleted the test262_wf_impr branch October 20, 2020 12:31

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

CI workflow improvements (mostly Test262) #889

CI workflow improvements (mostly Test262) #889

Razican commented Oct 18, 2020 •

edited

Loading

codecov bot commented Oct 18, 2020 •

edited

Loading

Razican commented Oct 19, 2020

HalidOdat left a comment

Razican commented Oct 19, 2020 •

edited

Loading

HalidOdat commented Oct 19, 2020 •

edited

Loading

Razican commented Oct 19, 2020

HalidOdat commented Oct 19, 2020

github-actions bot commented Oct 19, 2020 •

edited

Loading

Razican commented Oct 19, 2020

HalidOdat commented Oct 19, 2020

Razican commented Oct 19, 2020

Razican commented Oct 19, 2020

CI workflow improvements (mostly Test262) #889

CI workflow improvements (mostly Test262) #889

Conversation

Razican commented Oct 18, 2020 • edited Loading

codecov bot commented Oct 18, 2020 • edited Loading

Codecov Report

Razican commented Oct 19, 2020

HalidOdat left a comment

Choose a reason for hiding this comment

Razican commented Oct 19, 2020 • edited Loading

HalidOdat commented Oct 19, 2020 • edited Loading

Razican commented Oct 19, 2020

HalidOdat commented Oct 19, 2020

github-actions bot commented Oct 19, 2020 • edited Loading

Test262 conformance changes:

Razican commented Oct 19, 2020

HalidOdat commented Oct 19, 2020

Razican commented Oct 19, 2020

Razican commented Oct 19, 2020

Razican commented Oct 18, 2020 •

edited

Loading

codecov bot commented Oct 18, 2020 •

edited

Loading

Razican commented Oct 19, 2020 •

edited

Loading

HalidOdat commented Oct 19, 2020 •

edited

Loading

github-actions bot commented Oct 19, 2020 •

edited

Loading