Skip to content
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

Retrying to submit active-selection-045.html and a support file #24726

Closed
wants to merge 2 commits into from

Conversation

TalbotG
Copy link
Contributor

@TalbotG TalbotG commented Jul 23, 2020

active-selection-045.html (with a font size of 80px)

support/100x100-red.png

This is the bottom half of
#24714

@TalbotG TalbotG requested review from fantasai and frivoal and removed request for frivoal and plinss July 23, 2020 18:16
@TalbotG TalbotG assigned TalbotG and unassigned frivoal Jul 23, 2020
@wpt-pr-bot
Copy link
Collaborator

There are no reviewers for this pull request. Please reach out on W3C's irc server (irc.w3.org, port 6665) on channel #testing (web client) to get help with this. Thank you!

@TalbotG
Copy link
Contributor Author

TalbotG commented Jul 23, 2020

Dear fellow advanced GitHub colleagues @web-platform-tests/admins ,

Please help me understand why my test active-selection-045.html fails and fails with integration check wpt-chrome-dev-stability . I have been trying, re-trying and re-re-trying persistently to do so in
#24723
and in here
#24726

I have lots of questions right now ... but let's start with why active-selection-045.html is too much for wpt-chrome-dev-stability CI.

Thank you,

Gérard

@jgraham
Copy link
Contributor

jgraham commented Jul 24, 2020

Test Subtest Results Messages
/css/css-pseudo/active-selection-045.html FAIL: 2/10, PASS: 8/10

@jgraham
Copy link
Contributor

jgraham commented Jul 24, 2020

I strongly suspect this should use reftest-wait to ensure the script runs before the screenshot is taken.

@stephenmcgruer
Copy link
Contributor

I strongly suspect this should use reftest-wait to ensure the script runs before the screenshot is taken.

Agreed. Loading the test in a browser, there is a flicker of a frame drawn with the red-line existing before it disappears. @TalbotG , https://web-platform-tests.org/writing-tests/reftests.html#controlling-when-comparison-occurs documents the reftest-wait class, and you can look at a test like css/css-pseudo/marker-content-013.html for an example of it being used.

@TalbotG
Copy link
Contributor Author

TalbotG commented Jul 24, 2020

@jgraham , @stephenmcgruer ,

I have added the reftest-wait class to the root element in active-selection-045.html test in
#24714
[Edit: I have added a script instruction to remove class reftest-wait after script run a few min ago. Thank you jgraham.]
I have never use reftest-wait before and never thought this would be necessary here. But it makes sense, furthermore and especially if you see "a flicker of a frame drawn with the red-line existing before it disappears" ... which I do not.

What bugs me the most is that when a integration check reports a failure, there is no way for me to read some kind of error message describing somehow what failed or what was wrong or giving me some clue. So, I can not develop an autonomy, an independence with regards to GitHub and Continuous Integration checks. Just look:
#24710
#24714
#24715
#24723
#24726
are all about or related to active-selection-045.html test failure ... and even @fantasai could not figure out either what was wrong with 045 test.

@TalbotG
Copy link
Contributor Author

TalbotG commented Jul 24, 2020

Let's say I submit 10 tests in a PR. An integration checks fails. Ideally, an error message somewhere would inform about which test(s) has/have caused the failure. If, say, 2 tests have caused 1 CI check to fail, then I would still be able to re-submit a PR with the 8 "successful" tests. The way it is right now, I am not informed of which test(s) fail(s) and no clue given as to why it/they fail.

@stephenmcgruer
Copy link
Contributor

What bugs me the most is that when a integration check reports a failure, there is no way for me to read some kind of error message describing somehow what failed or what was wrong or giving me some clue. So, I can not develop an autonomy, an independence with regards to GitHub and Continuous Integration checks. Just look:

I think there are two levels to this. Firstly, there is accessing the details of the check, and secondly there is understanding what the failure means.

For the first, the details are available today, but we do also have long-term plans to make them more easily accessible. Today, it requires clicking through three levels: 'details' on GitHub, then 'View task in Taskcluster', then 'View Live Log':

Screenshot 2020-07-24 at 13 04 48

Screenshot 2020-07-24 at 13 04 57

Screenshot 2020-07-24 at 13 05 05

In the future, we intend to extract information from the logs and put it in the first GitHub Checks output object (the area where today it just says "Verify that all tests affected by a pull request are stable when executed in chrome.").

The second part of the question is: how do you understand from the checks what has gone wrong. This is a much harder problem, and it's not clear to me how far along that road we should go. Today, the logs will tell you (a) what the inconsistency was (**FAIL: 2/10, PASS: 8/10** for your test), and (b) it will above have logged the result for each test along the way. I note that for reftests we don't seem to log the base64-encoded png of the screenshots, perhaps we should do that (or perhaps we should store the pngs in the artifacts space, though I'm not sure how much overall space we have available there). Going much further than this seems like a huge investment in guessing what the test may be doing wrong - there are so, so many ways to cause flakes :)

@TalbotG
Copy link
Contributor Author

TalbotG commented Jul 24, 2020

The details are available (...) through 'details' on GitHub, then 'View task in Taskcluster', then 'View Live Log': just finding this was not obvious to me. And once in View Live Log, I could not scroll that page with the vertical scrollbar. I only could roll the mousewheel to get into the file... once I noticed that I could scroll that 'View Live Log' page with the mousewheel.

The re-run link in the Details page: it did not work... or I could not make it work ... or it was re-running but I could not be sure of it (no visual feedback).

etc

@TalbotG
Copy link
Contributor Author

TalbotG commented Jul 24, 2020

I must close this PR and delete this branch now that
#24714
has merged.

@TalbotG TalbotG closed this Jul 24, 2020
@TalbotG TalbotG deleted the CSS4Pseudo-GT-PR32 branch July 24, 2020 17:36
@stephenmcgruer
Copy link
Contributor

No disagreement to any of that; that is exactly what #15412 covers. We're aware that this is painful, and we're sorry and we'd like to fix it, but we have limited bandwidth. Slowly making progress in that direction - the big hurdle of moving to Taskcluster Checks is done and now we 'just' need to rewrite part of the stability checker to output formatted data that GitHub can read.

@TalbotG
Copy link
Contributor Author

TalbotG commented Jul 24, 2020

Stephen,

If/when I see a vertical scrollbar in a webpage, then I should (and I expect) to be able to scroll it with [PgDn]/[PgUp] or navigation up/down arrow keys. There is a counter-intuitive, disorienting visual feedback in that View Live Log page.

If a particular CI check fails, then I wish (and wished) I would be informed somehow/somewhere about which test(s) in my PR is causing issues or problems from any of the 'details' or 'View task in Taskcluster' or 'View Live Log' "hoops" (sorry if my english is awkward here).

How to convey a descriptive error message to the submitter is indeed a huge complex task. I have unfortunately no constructive suggestion or idea on that specific issue.

@stephenmcgruer
Copy link
Contributor

If/when I see a vertical scrollbar in a webpage, then I should (and I expect) to be able to scroll it with [PgDn]/[PgUp] or navigation up/down arrow keys. There is a counter-intuitive, disorienting visual feedback in that View Live Log page.

Yeah, the Taskcluster UI is... a little funky. I have also found trouble getting it to scroll. Feel free to open an issue https://github.com/taskcluster/taskcluster/issues/

If a particular CI check fails, then I wish (and wished) I would be informed somehow/somewhere about which test(s) in my PR is causing issues or problems from any of the 'details' or 'View task in Taskcluster' or 'View Live Log' "hoops" (sorry if my english is awkward here).

Yep, no disagreement. Long term I'm aiming for 1 hop ('details') and that's it. I personally would also like a bot that comments on the PR with details, but that's contentious around here (a lot of folks object to bot comments).

@TalbotG
Copy link
Contributor Author

TalbotG commented Jul 24, 2020

the logs will tell you (a) what the inconsistency was (FAIL: 2/10, PASS: 8/10 for your test)

I did not see this. I still today (and yesterday and 2 days ago) do not see this. I do not know where you see/saw this particular information ! I do see "2 failing and 17 successful checks".

@stephenmcgruer
Copy link
Contributor

At the bottom of the logs for wpt-chrome-dev-stability:

image

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

Successfully merging this pull request may close these issues.

5 participants