Closed
Bug 1000870
Opened 11 years ago
Closed 9 years ago
Tests for check support Pointer Events
Categories
(Core :: DOM: Events, defect)
Core
DOM: Events
Tracking
()
RESOLVED
FIXED
mozilla48
Tracking | Status | |
---|---|---|
firefox48 | --- | fixed |
People
(Reporter: alessarik, Assigned: alessarik)
References
(Depends on 1 open bug)
Details
Attachments
(3 files, 20 obsolete files)
295.44 KB,
patch
|
smaug
:
review+
|
Details | Diff | Splinter Review |
1.48 KB,
patch
|
smaug
:
review+
|
Details | Diff | Splinter Review |
225.85 KB,
patch
|
smaug
:
review+
|
Details | Diff | Splinter Review |
FireFox should have internal auto tests for check support Pointer Events during build
Assignee | ||
Comment 1•11 years ago
|
||
Some of test to check support Pointer Events
Attachment #8411798 -
Flags: feedback?(romaxa)
Attachment #8411798 -
Flags: feedback?(nicklebedev37)
Attachment #8411798 -
Flags: feedback?(bugs)
Comment 2•11 years ago
|
||
So this is waiting for w3c tests to be reviewed.
Updated•11 years ago
|
Component: Untriaged → DOM: Events
Product: Firefox → Core
Comment 3•11 years ago
|
||
Comment on attachment 8411798 [details] [diff] [review] off_tests_ver5.diff Do you have try build with all these tests passing? Overall it looks great to me.
Attachment #8411798 -
Flags: feedback?(romaxa) → feedback+
Assignee | ||
Comment 4•11 years ago
|
||
(In reply to Oleg Romashin (:romaxa) from comment #3) > Do you have try build with all these tests passing? https://tbpl.mozilla.org/?tree=Try&rev=3b3cfeb83a2b But most of tests were finished by timeout without any results.
Comment 5•11 years ago
|
||
So what parts of the tests are coming from w3c test suite (which is still being reviewed, I believe)?
Comment 6•11 years ago
|
||
Comment on attachment 8411798 [details] [diff] [review] off_tests_ver5.diff Review of attachment 8411798 [details] [diff] [review]: ----------------------------------------------------------------- We have similar bug about pointer events tests: 966961. I believe we need to either tie them or close one of them as duplicate.
Attachment #8411798 -
Flags: feedback?(nicklebedev37) → feedback+
Comment 7•10 years ago
|
||
Are all the w3c tests now reviewed?
Assignee | ||
Comment 8•10 years ago
|
||
All test were got from https://github.com/w3c/web-platform-tests/tree/master/pointerevents I made minimal changes (to support mochitest system) Some test were passed succesfully. Some were failed. I try to check fails. https://tbpl.mozilla.org/?tree=Try&rev=43d29e9c5602
Attachment #8411798 -
Attachment is obsolete: true
Attachment #8411798 -
Flags: feedback?(bugs)
Attachment #8446538 -
Flags: review?(peterv)
Attachment #8446538 -
Flags: review?(jst)
Attachment #8446538 -
Flags: review?(bugs)
Updated•10 years ago
|
Attachment #8446538 -
Flags: review?(peterv)
Attachment #8446538 -
Flags: review?(jst)
Comment 9•10 years ago
|
||
Comment on attachment 8446538 [details] [diff] [review] off_tests_ver6.diff So the tests don't pass on try.
Attachment #8446538 -
Flags: review?(bugs)
Comment 10•10 years ago
|
||
Also, would it be possible to get the diff for the changes you've made to the original files? That would make reviewing _a_lot_ easier.
Assignee | ||
Comment 11•10 years ago
|
||
+ Some changes in test were added
Attachment #8446538 -
Attachment is obsolete: true
Attachment #8451027 -
Flags: review?(jonas)
Attachment #8451027 -
Flags: review?(bzbarsky)
Attachment #8451027 -
Flags: review?(bugs)
Assignee | ||
Comment 12•10 years ago
|
||
+ Some changes which needs for correct testing
Attachment #8451033 -
Flags: review?(jonas)
Attachment #8451033 -
Flags: review?(bzbarsky)
Attachment #8451033 -
Flags: review?(bugs)
Attachment #8451033 -
Flags: feedback?(oleg.romashin)
Attachment #8451033 -
Flags: feedback?(nicklebedev37)
Assignee | ||
Comment 13•10 years ago
|
||
https://tbpl.mozilla.org/?tree=Try&rev=a6fe819e2e16
Assignee | ||
Comment 14•10 years ago
|
||
Original official tests from W3C at 2014.07.01
Attachment #8451027 -
Flags: review?(jonas)
Attachment #8451033 -
Flags: review?(jonas)
Updated•10 years ago
|
Attachment #8451027 -
Flags: review?(bzbarsky)
Updated•10 years ago
|
Attachment #8451033 -
Flags: review?(bzbarsky)
Updated•10 years ago
|
Attachment #8451033 -
Flags: review?(bugs) → review+
Comment 15•10 years ago
|
||
Comment on attachment 8451027 [details] [diff] [review] off_tests_ver7.diff >-MOCHITEST_MANIFESTS += ['test/mochitest.ini'] >+MOCHITEST_MANIFESTS += [ >+# 'test/mochitest.ini', Why this? But I can't really review this all. Please provide a diff which shows which changes you have made to the original w3c tests.
Attachment #8451027 -
Flags: review?(bugs) → review-
Comment 16•10 years ago
|
||
Comment on attachment 8452251 [details]
original_official_tests.diff
So if you have these, then do diff comparing all the files in this patch
to the patch you asked a review for.
Or interdiff of this patch and the other patch might work too.
Assignee | ||
Comment 17•10 years ago
|
||
(In reply to Olli Pettay [:smaug] from comment #15) > >-MOCHITEST_MANIFESTS += ['test/mochitest.ini'] > >+MOCHITEST_MANIFESTS += [ > >+# 'test/mochitest.ini', > Why this? Only for testing. In finish version will be at normal state.
Assignee | ||
Comment 18•10 years ago
|
||
Initial state of tests. TRY: https://tbpl.mozilla.org/?tree=Try&rev=524acfdfdd22
Attachment #8452251 -
Attachment is obsolete: true
Attachment #8455950 -
Flags: review?(bugs)
Attachment #8455950 -
Flags: feedback?(oleg.romashin)
Attachment #8455950 -
Flags: feedback?(nicklebedev37)
Comment 19•10 years ago
|
||
Comment on attachment 8455950 [details] [diff] [review] original_official_tests.diff I really need the interdiff, in other words I should review only the changes you've made and I could rely on the tests from w3c being right (mbrubeck et al have reviewed those).
Attachment #8455950 -
Flags: review?(bugs)
Assignee | ||
Comment 20•10 years ago
|
||
+ Changes: Patch is difference over original_official_tests.diff
Attachment #8451027 -
Attachment is obsolete: true
Attachment #8463978 -
Flags: review?(bugs)
Comment 21•10 years ago
|
||
Comment on attachment 8463978 [details] [diff] [review] off_tests_ver8.diff > test(function() { >- assert_true(event.button == -1, "If mouse buttons are released button attribute is -1") >+ assert_equals(event.button, -1, "If mouse buttons are released button attribute is -1") > }, "If mouse buttons are released button attribute is -1"); > test(function() { >- assert_true(event.buttons == 0, "If mouse buttons are released buttons attribute is 0") >+ assert_equals(event.buttons, 0, "If mouse buttons are released buttons attribute is 0") > }, "If mouse buttons are released buttons attribute is 0"); Why these changes? >@@ -79,15 +85,15 @@ > function run() { > on_event(target0, "pointerover", function (event) { > log("pointerover", target0); > if(isPointerCapture) { > test_pointerover_capture.done(); > if (!isRelatedTargetValueTested) { > test(function() { >- assert_true(event.relatedTarget==null, "relatedTarget is null when the capture is set") >+ assert_equals(event.relatedTarget, null, "relatedTarget is null when the capture is set") I don't understand this change. Those fixed, I think we could get this in. (We may want to change the setup at some point, but w3c harness lacks some features.)
Attachment #8463978 -
Flags: review?(bugs) → review+
Assignee | ||
Comment 22•10 years ago
|
||
(In reply to Olli Pettay [:smaug] from comment #21) > Why these changes? > I don't understand this change. In this case we can read more information in logs. For example, we can see actual result and expected value. Without this changes, we can see only "FAIL" message without details of error.
Comment 23•10 years ago
|
||
But we want as few changes as possible, so that we can update the test easily if there are changes to w3c tests.
Updated•10 years ago
|
Attachment #8455950 -
Flags: feedback?(nicklebedev37)
Updated•10 years ago
|
Attachment #8451033 -
Flags: feedback?(nicklebedev37)
Assignee | ||
Comment 25•10 years ago
|
||
Unfortunately, I have no update for this at this time. But we should get new version of tests from official repository. There are many changes in repo. But original_official_tests.diff is very older.
Comment 26•10 years ago
|
||
Maksim, would be good to get the tests landed.
Assignee | ||
Comment 27•10 years ago
|
||
(In reply to Olli Pettay [:smaug] from comment #26) > Maksim, would be good to get the tests landed. I will try to synchronize tests with latest changes in official repo.
Blocks: 960316
Assignee | ||
Comment 28•10 years ago
|
||
Test suite from official repo at 2015.03.01
Attachment #8455950 -
Attachment is obsolete: true
Attachment #8455950 -
Flags: feedback?(oleg.romashin)
Attachment #8586103 -
Flags: review?(bugs)
Assignee | ||
Comment 29•10 years ago
|
||
Comment on attachment 8451033 [details] [diff] [review] testing_features_ver1.diff This changes already were pushed in another bug.
Attachment #8451033 -
Attachment is obsolete: true
Attachment #8451033 -
Flags: feedback?(oleg.romashin)
Comment 30•10 years ago
|
||
Comment on attachment 8586103 [details] [diff] [review] original_official_tests_ver2.diff I don't see how this patch makes us to run the tests automatically. Are these the manual-only tests? If so, I don't think we need those in the tree. Or do I misunderstand something here now.
Attachment #8586103 -
Flags: review?(bugs) → review-
Comment 31•10 years ago
|
||
So we need tests, but we need tests to be run automatically.
Assignee | ||
Comment 32•10 years ago
|
||
+ Add posibility to send no-pressed button in event
Attachment #8587496 -
Flags: review?(bugs)
Assignee | ||
Comment 33•10 years ago
|
||
(In reply to Olli Pettay [:smaug] from comment #30) > I don't see how this patch makes us to run the tests automatically. > Are these the manual-only tests? If so, I don't think we need those in the tree. > > Or do I misunderstand something here now. You always asked (in comment 10, comment 15, comment 16, comment 19) to provide two patches: official patch and patch with small differencies.
Assignee | ||
Comment 34•10 years ago
|
||
+ Each test now has own support file (To decrease changes in official tests and turn on pointer events in some tests before loading page) - Script testharnessreport.js was removed from each official test. (Each official test is loading in iframe. In this case testharnessreport.js generates exception).
Attachment #8463978 -
Attachment is obsolete: true
Attachment #8587510 -
Flags: review?(bugs)
Updated•10 years ago
|
Attachment #8587496 -
Flags: review?(bugs) → review+
Comment 35•10 years ago
|
||
Comment on attachment 8587510 [details] [diff] [review] off_tests_ver9.diff So I need this to be split to two patches. First one patch containing all the tests without any changes. That I would r+ pretty much automatically (assuming there is the other patch to review too) (perhaps attachment 8586103 [details] [diff] [review] is that first patch?) then another patch on top of that adding all the changes need to run this stuff automatically. I would expect the second patch to be a lot smaller than the first one.
Attachment #8587510 -
Flags: review?(bugs) → review-
Comment 36•10 years ago
|
||
(In reply to Maksim Lebedev from comment #33) > You always asked (in comment 10, comment 15, comment 16, comment 19) > to provide two patches: official patch and patch with small differencies. Exactly. And I saw only the official tests -patch in the queue, not the patch which does the changes. I'd like to see them both.
Assignee | ||
Comment 37•10 years ago
|
||
(In reply to Olli Pettay [:smaug] from comment #35) > So I need this to be split to two patches. I provided three patches for more convinience. > First one patch containing all the tests without any changes. It is "original_official_tests.diff" with some version. > That I would r+ pretty much automatically That patch got r-, unfortunately. > then another patch on top of that adding all the changes need to run this stuff automatically. It is "off_tests.diff" with some version. > I would expect the second patch to be a lot smaller than the first one. Unfortunately, it has a lot of additionally info. (For example, the largest file in folder is "mochitest.ini". It is situated in "off_tests" patch). Also, another changes were discribed in comment 34. (In reply to Olli Pettay [:smaug] from comment #36) > I'd like to see them both. Both patches were provided. And also third patch was provided with changes in CPP source code for more comfortable review.
Comment 38•10 years ago
|
||
(In reply to Maksim Lebedev from comment #37) > It is "original_official_tests.diff" with some version. > > That I would r+ pretty much automatically > That patch got r-, unfortunately. Because at that time there wasn't another patch to make those tests automatically testable. > > then another patch on top of that adding all the changes need to run this stuff automatically. > It is "off_tests.diff" with some version. Ahaa, now I'm starting to see your setup ok, reviewing again
Comment 39•10 years ago
|
||
Comment on attachment 8586103 [details] [diff] [review] original_official_tests_ver2.diff rs+ (rubberstamp)
Attachment #8586103 -
Flags: review- → review+
Updated•10 years ago
|
Attachment #8587510 -
Flags: review- → review?(bugs)
Assignee | ||
Comment 40•10 years ago
|
||
(In reply to Olli Pettay [:smaug] from comment #38) > Because at that time there wasn't another patch to make those tests automatically testable. There were some issues with work of auto test system. That issues now are discribed in bug 1150091.
Comment 41•10 years ago
|
||
Do we need bug 1150091 fixed before the patches in this bug can land? Or if bug 1150091 doesn't get fixed soon, should the patch in this bug be changed?
Comment 42•10 years ago
|
||
Comment on attachment 8587510 [details] [diff] [review] off_tests_ver9.diff Apparently a new patch coming.
Attachment #8587510 -
Flags: review?(bugs)
Assignee | ||
Comment 43•10 years ago
|
||
+ Update code according with GetButtonsFlagForButton() -> nsContentUtils::GetButtonsFlagForButton()
Assignee: nobody → alessarik
Attachment #8587496 -
Attachment is obsolete: true
Status: UNCONFIRMED → NEW
Ever confirmed: true
Attachment #8606376 -
Flags: review?(bugs)
Assignee | ||
Comment 44•10 years ago
|
||
+ Four tests were disabled for future investigation + Some issues in tests were resolved Suggestions and comments and objections are very welcome.
Attachment #8587510 -
Attachment is obsolete: true
Flags: needinfo?(alessarik)
Attachment #8606386 -
Flags: review?(bugs)
Attachment #8606386 -
Flags: feedback?(jmathies)
Assignee | ||
Comment 45•10 years ago
|
||
Full TRY-server testing: https://treeherder.mozilla.org/#/jobs?repo=try&revision=408532c91a84 Small TRY-server testing: https://treeherder.mozilla.org/#/jobs?repo=try&revision=5fdf4c8e64be AND another one: https://treeherder.mozilla.org/#/jobs?repo=try&revision=71c74865d225
Comment 46•10 years ago
|
||
Comment on attachment 8606376 [details] [diff] [review] testing_feauters_ver3.diff actually, could you add a new flag to buttonsFlag, eNoButtonFlag = 0x00, and return that in GetButtonsFlagForButton. It would make the code a bit easier to understand. With that change, r+
Attachment #8606376 -
Flags: review?(bugs) → review+
Comment 47•10 years ago
|
||
Comment on attachment 8606386 [details] [diff] [review] off_tests_ver10.diff >+// Helper function to send PointerEvent with different parameters >+function sendPointerEvent(int_win, elemId, pointerType, inputSource, params) { >+ var elem = int_win.document.getElementById(elemId); >+ if(!!elem) { >+ var rect = elem.getBoundingClientRect(); >+ var eventObj = {type: pointerType, inputSource: inputSource}; >+ if(params && "button" in params) >+ eventObj.button = params.button; >+ if(params && "isPrimary" in params) >+ eventObj.isPrimary = params.isPrimary; >+ else if(SpecialPowers.Ci.nsIDOMMouseEvent.MOZ_SOURCE_MOUSE == inputSource) You don't need SpecialPowers here. Just MouseEvent.MOZ_SOURCE_MOUSE. Similar also elsewhere. >+ eventObj.isPrimary = true; >+ console.log(elemId, eventObj); >+ var salt = ("pointermove" == pointerType) ? 1 : 2; I don't understand this. how could pointerType be "pointermove"? So, I guess pointerType should be renamed to pointerEventType Mostly rs+ (rubberstamp+) manual tests won't be run automatically, but better to have them in tree.
Attachment #8606386 -
Flags: review?(bugs)
Attachment #8606386 -
Flags: review+
Attachment #8606386 -
Flags: feedback?(jmathies)
Comment 48•10 years ago
|
||
Make sure to run the tests couple of times on tryserver before the patch is pushed to mozilla-inbound.
Assignee | ||
Comment 49•10 years ago
|
||
+ Added WidgetMouseEvent::buttonsFlag::eNoButtonFlag
Attachment #8606376 -
Attachment is obsolete: true
Attachment #8607535 -
Flags: review?(bugs)
Assignee | ||
Comment 50•10 years ago
|
||
+ Changed pointerType -> pointerEventType in sendPointerEvent() + Changed SpecialPowers.Ci.nsIDOMMouseEvent.MOZ_SOURCE_* -> MouseEvent.MOZ_SOURCE_* Suggestions and comments and objections are very welcome.
Attachment #8606386 -
Attachment is obsolete: true
Attachment #8607539 -
Flags: review?(bugs)
Assignee | ||
Comment 51•10 years ago
|
||
Testing: https://treeherder.mozilla.org/#/jobs?repo=try&revision=83d5ec7ba688 Testing: https://treeherder.mozilla.org/#/jobs?repo=try&revision=14588acd5588
Comment 52•10 years ago
|
||
Comment on attachment 8607539 [details] [diff] [review] off_tests_ver11.diff interdiff to the previous version of the patch would have been nice ;) Mostly rs+ based on the comment about the changes.
Attachment #8607539 -
Flags: review?(bugs) → review+
Updated•10 years ago
|
Attachment #8607535 -
Flags: review?(bugs) → review+
Assignee | ||
Comment 53•9 years ago
|
||
Testing: https://treeherder.mozilla.org/#/jobs?repo=try&revision=80100e4c793d
Assignee | ||
Comment 54•9 years ago
|
||
Testing: https://treeherder.mozilla.org/#/jobs?repo=try&revision=c147081a001b
Assignee | ||
Comment 55•9 years ago
|
||
+ Added small changes at "pointerevent_capture_mouse-manual.html" test Suggestions and comments and objections are very welcome.
Attachment #8607539 -
Attachment is obsolete: true
Attachment #8623612 -
Flags: review?(bugs)
Comment 56•9 years ago
|
||
Comment on attachment 8623612 [details] [diff] [review] off_tests_ver12.diff What is the status of this bug? What is blocking landing patches?
Attachment #8623612 -
Flags: review?(bugs) → review+
Assignee | ||
Comment 57•9 years ago
|
||
(In reply to Olli Pettay [:smaug] from comment #56) > What is the status of this bug? What is blocking landing patches? Looks like a most part of tests have stable results, so we can land this changes.
Assignee | ||
Comment 58•9 years ago
|
||
If there are no objections, I put checkin-needed flag.
Keywords: checkin-needed
Comment 59•9 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/03655a6288c2 https://hg.mozilla.org/integration/mozilla-inbound/rev/ed0707ffd26f https://hg.mozilla.org/integration/mozilla-inbound/rev/403078fbf2e2
Keywords: checkin-needed
Assignee | ||
Comment 60•9 years ago
|
||
Testing: https://treeherder.mozilla.org/#/jobs?repo=try&revision=c151163afa96
Comment 61•9 years ago
|
||
sorry had to back this out for test failures like https://treeherder.mozilla.org/logviewer.html#?job_id=10960451&repo=mozilla-inbound
Flags: needinfo?(alessarik)
Comment 62•9 years ago
|
||
Backout: https://hg.mozilla.org/integration/mozilla-inbound/rev/a624cf1be291 https://hg.mozilla.org/integration/mozilla-inbound/rev/046956f23712
Comment 63•9 years ago
|
||
Backout: https://hg.mozilla.org/integration/mozilla-inbound/rev/80a68c35ea1a
Assignee | ||
Comment 64•9 years ago
|
||
Testing: https://treeherder.mozilla.org/#/jobs?repo=try&revision=ebe6c185966d
Assignee | ||
Comment 65•9 years ago
|
||
Testing: https://treeherder.mozilla.org/#/jobs?repo=try&revision=e99d0c82bc09
Assignee | ||
Comment 66•9 years ago
|
||
+ Added skipping two tests on linux platform Suggestions and comments and objections are very welcome.
Attachment #8623612 -
Attachment is obsolete: true
Attachment #8625739 -
Flags: review?(bugs)
Assignee | ||
Comment 67•9 years ago
|
||
Looks like test results from Comment 65 have no issues related with current patches.
Flags: needinfo?(cbook)
Comment 68•9 years ago
|
||
Any idea why that test might fail on linux?
Comment 69•9 years ago
|
||
Comment on attachment 8625739 [details] [diff] [review] off_tests_ver13.diff But we can fix that in a different bug to get these test landed sooner than later. Please file a followup to figure out those linux-only issues.
Attachment #8625739 -
Flags: review?(bugs) → review+
Assignee | ||
Comment 70•9 years ago
|
||
(In reply to Olli Pettay [:smaug] from comment #68) > Any idea why that test might fail on linux? I have no prepared environment on linux. At least at current time. Fails looks very strange because it happens unstable and probably only in debug mode. So maybe it need deeper investigation.
Assignee | ||
Comment 71•9 years ago
|
||
If there are no objections, I put checkin-needed flag.
Comment 72•9 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/6a730e15246e https://hg.mozilla.org/integration/mozilla-inbound/rev/80d8e9adb532 https://hg.mozilla.org/integration/mozilla-inbound/rev/7983770003a3
Keywords: checkin-needed
Comment 73•9 years ago
|
||
Backed out for the test failures noted in bug 1151152. https://hg.mozilla.org/mozilla-central/rev/716df1829f4f
Flags: needinfo?(cbook)
Comment 74•9 years ago
|
||
Permafail on B2G from the looks of it. https://treeherder.mozilla.org/logviewer.html#?job_id=11182535&repo=mozilla-inbound
Assignee | ||
Comment 75•9 years ago
|
||
Testing: https://treeherder.mozilla.org/#/jobs?repo=try&revision=9867fd5fd5fb
Assignee | ||
Comment 76•9 years ago
|
||
+ Skiped [test_pointerevent_capture_suppressing_mouse-manual.html] on B2G Emulator + Changes in [test_pointerevent_setpointercapture_inactive_button_mouse-manual.html] + Added some comment about bugs and issues Suggestions and comments and objections are very welcome.
Attachment #8625739 -
Attachment is obsolete: true
Attachment #8629357 -
Flags: review?(bugs)
Assignee | ||
Comment 77•9 years ago
|
||
Testing: https://treeherder.mozilla.org/#/jobs?repo=try&revision=c42d0338462b
Updated•9 years ago
|
Attachment #8629357 -
Flags: review?(bugs) → review+
Assignee | ||
Comment 78•9 years ago
|
||
Testing: https://treeherder.mozilla.org/#/jobs?repo=try&revision=d0eab88589b5
Assignee | ||
Comment 79•9 years ago
|
||
Testing: https://treeherder.mozilla.org/#/jobs?repo=try&revision=9fa2c75534ab
Assignee | ||
Comment 80•9 years ago
|
||
Testing: https://treeherder.mozilla.org/#/jobs?repo=try&revision=48848768c45b
Assignee | ||
Comment 81•9 years ago
|
||
Testing: https://treeherder.mozilla.org/#/jobs?repo=try&revision=c5ad3ca862ff
Assignee | ||
Comment 82•9 years ago
|
||
+ Skiped some tests on B2G ICS Emulator + Added small correcting changes into tests Suggestions and comments and objections are very welcome.
Attachment #8629357 -
Attachment is obsolete: true
Attachment #8631465 -
Flags: review?(bugs)
Comment 83•9 years ago
|
||
Comment on attachment 8631465 [details] [diff] [review] off_tests_ver15.diff Please explain those changes to the tests. Why we need them? But I can rs+ this anyhow.
Attachment #8631465 -
Flags: review?(bugs) → review+
Assignee | ||
Comment 84•9 years ago
|
||
(In reply to Olli Pettay [:smaug] from comment #83) > Please explain those changes to the tests. Why we need them? Issues were in test related with lostpointercapture events, because such events work as async I have added some syntetized pointermove events for better working.
Assignee | ||
Comment 85•9 years ago
|
||
If there are no objections, I put checkin-needed flag. Let's try to push it again.
Keywords: checkin-needed
Comment 86•9 years ago
|
||
(In reply to Maksim Lebedev from comment #85) > If there are no objections, I put checkin-needed flag. Let's try to push it > again. I object. Your last Try run shows failures on both Android and B2G debug. https://treeherder.mozilla.org/logviewer.html#?job_id=9173724&repo=try https://treeherder.mozilla.org/logviewer.html#?job_id=9175530&repo=try
Keywords: checkin-needed
Assignee | ||
Comment 87•9 years ago
|
||
(In reply to Ryan VanderMeulen [:RyanVM UTC-4] from comment #86) > I object. Your last Try run shows failures on both Android and B2G debug. Is there any construction for skip all tests on B2G ICS emulator or I should write "skip" word into each small test?
Flags: needinfo?(ryanvm)
Comment 88•9 years ago
|
||
You can add |skip-if = toolkit == 'gonk'| to the [DEFAULT] section of the manifest to skip all tests on B2G emulator builds. If you want to skip on only debug, it'd be |skip-if = (toolkit == 'gonk' && debug)|. Note that there are intermittent Android failures as well. Frankly, I'd like to see multiple (10+) green retriggers before landing this to avoid new intermittents. Also, can you please try to use more targeted Try pushes for this instead of running all builds and all tests across all platforms? Doing so uses over 300hr of machine time.
Flags: needinfo?(ryanvm)
Assignee | ||
Comment 89•9 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=4cb55476c10f
Assignee | ||
Comment 90•9 years ago
|
||
Testing: https://treeherder.mozilla.org/#/jobs?repo=try&revision=dd9aadf9697c
Assignee | ||
Comment 91•9 years ago
|
||
+ Moved "skip-if" statement from several separated tests into default section Suggestions and comments and objections are very welcome.
Attachment #8631465 -
Attachment is obsolete: true
Attachment #8641713 -
Flags: review?(bugs)
Assignee | ||
Comment 92•9 years ago
|
||
"Debug B2G ICS Emulator" is very strange configuration with unstable behavior. So all pointer events tests were disabled on that configuration until happy future.
Updated•9 years ago
|
Attachment #8641713 -
Flags: review?(bugs) → review+
Assignee | ||
Comment 93•9 years ago
|
||
If there are no objections, I put checkin-needed flag. Let's try to push it again.
Keywords: checkin-needed
Comment 94•9 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/4b6f2efbdc36 https://hg.mozilla.org/integration/mozilla-inbound/rev/427cfffcb5b7 https://hg.mozilla.org/integration/mozilla-inbound/rev/5490127f9b99
Keywords: checkin-needed
Comment 95•9 years ago
|
||
sorry had to back this out for test failures like https://treeherder.mozilla.org/logviewer.html#?job_id=12416096&repo=mozilla-inbound
Flags: needinfo?(alessarik)
Comment 96•9 years ago
|
||
Backout: https://hg.mozilla.org/integration/mozilla-inbound/rev/a8147b48ea8d https://hg.mozilla.org/integration/mozilla-inbound/rev/402b0919ede8 https://hg.mozilla.org/integration/mozilla-inbound/rev/7cb778938f3c
Assignee | ||
Comment 97•9 years ago
|
||
+ Skiped tests on Android in Debug mode Suggestions and comments and objections are very welcome.
Attachment #8641713 -
Attachment is obsolete: true
Attachment #8699870 -
Flags: review?(bugs)
Assignee | ||
Comment 98•9 years ago
|
||
Testing: https://treeherder.mozilla.org/#/jobs?repo=try&revision=159492c4f1c0 Looks like, there are no failing tests which are related with pointer events.
Comment 99•9 years ago
|
||
Comment on attachment 8699870 [details] [diff] [review] off_tests_ver17.diff Could you please provide interdiff.
Assignee | ||
Comment 100•9 years ago
|
||
(In reply to Olli Pettay [:smaug] from comment #99) > Could you please provide interdiff. Interdiff between off_tests_ver16 and off_tests_ver17 is very simple: > [DEFAULT] > -skip-if = debug && (toolkit == 'gonk') # Bug 1178701 - Issue on 'B2G ICS Emulator' > +skip-if = debug && ((toolkit == 'gonk') || (os == 'android')) # Bug 1178701 - Issue on 'B2G ICS Emulator' and 'Android' > support-files =
Updated•9 years ago
|
Attachment #8699870 -
Flags: review?(bugs) → review+
Assignee | ||
Comment 101•9 years ago
|
||
Today it is good time to finish this long-running task :-) If there are no objections, I put checkin-needed flag. Let's try to push it again.
Keywords: checkin-needed
Comment 102•9 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/44ac8de96a09 https://hg.mozilla.org/integration/mozilla-inbound/rev/d206a3fd7433 https://hg.mozilla.org/integration/mozilla-inbound/rev/a0c6e3693039
Keywords: checkin-needed
Comment 103•9 years ago
|
||
sorry had to back this out for test failures in https://treeherder.mozilla.org/logviewer.html#?job_id=19208928&repo=mozilla-inbound
Comment 104•9 years ago
|
||
Backout: https://hg.mozilla.org/integration/mozilla-inbound/rev/a53a255834e7 https://hg.mozilla.org/integration/mozilla-inbound/rev/28994fda3259 https://hg.mozilla.org/integration/mozilla-inbound/rev/ae7f6845ecc6
Assignee | ||
Comment 105•9 years ago
|
||
Add skip for one test.
> [test_pointerevent_setpointercapture_inactive_button_mouse-manual.html]
> + skip-if = debug && (os == 'linux') && e10s
Attachment #8699870 -
Attachment is obsolete: true
Attachment #8713517 -
Flags: review?(bugs)
Assignee | ||
Comment 106•9 years ago
|
||
Testing: https://treeherder.mozilla.org/#/jobs?repo=try&revision=dcb568c7683d
Comment 107•9 years ago
|
||
Comment on attachment 8713517 [details] [diff] [review] off_tests_ver18.diff r+, but please do still another try run using all the platforms. This has been backed out so many times
Attachment #8713517 -
Flags: review?(bugs) → review+
Assignee | ||
Comment 108•9 years ago
|
||
Unfortunately, it is needed to skip several more tests.
Difference:
> -skip-if = debug && ((toolkit == 'gonk') || (os == 'android'))
> +skip-if = (toolkit == 'gonk') || (os == 'android')
Attachment #8713517 -
Attachment is obsolete: true
Attachment #8715278 -
Flags: review?(bugs)
Updated•9 years ago
|
Attachment #8715278 -
Flags: review?(bugs) → review+
Comment 109•9 years ago
|
||
Do you have a link to the failures on Android?
Assignee | ||
Comment 110•9 years ago
|
||
(In reply to Olli Pettay [:smaug] (high review load) from comment #109) > Do you have a link to the failures on Android? Yes: https://treeherder.mozilla.org/#/jobs?repo=try&revision=1d4a20419521 Looks like Android-opt didn't run all of that test for a long time before latest time. So, test-run-results looks better after latest changes. https://treeherder.mozilla.org/#/jobs?repo=try&revision=d3c9a3312e60
Assignee | ||
Comment 111•9 years ago
|
||
So, if there are no objections, I put checkin-needed flag. Let's try to push it again. I hope it will be the latest iteration of this bug :-)
Keywords: checkin-needed
Comment 112•9 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/07d6feffedb9 https://hg.mozilla.org/integration/mozilla-inbound/rev/76c66d5a8196 https://hg.mozilla.org/integration/mozilla-inbound/rev/c8273997b141
Keywords: checkin-needed
Comment 113•9 years ago
|
||
It seems rather unfortunate that we seem to have added a large number of testharness.js tests to a Mozilla-only location rather than testing/web-platform/tests where they would be upstreamed to wpt and shared with other vendors to improve interop. Is there any good reason these tests can't be moved to web-platform-tests?
Comment 114•9 years ago
|
||
If some subset of the tests are using mozilla-only APIs testing/web-platform/mozilla is still a better location because then we know that the tests are there when we are looking for things to convert later when the standard harness has more features.
Comment 115•9 years ago
|
||
had to back this out for test failures in https://treeherder.mozilla.org/logviewer.html#?job_id=21127288&repo=mozilla-inbound
Comment 116•9 years ago
|
||
Backout: https://hg.mozilla.org/integration/mozilla-inbound/rev/37904e50fa46 https://hg.mozilla.org/integration/mozilla-inbound/rev/6b49048a0f7a https://hg.mozilla.org/integration/mozilla-inbound/rev/bdd28f4c1f0d
Assignee | ||
Comment 117•9 years ago
|
||
Difference for test setpointercapture_inactive_button_mouse > - skip-if = debug && (os == 'linux') && e10s # Bug 1180188 - Issue on Linux > + skip-if = (os == 'linux') && e10s # Bug 1180188 - Issue on Linux Testing: https://treeherder.mozilla.org/#/jobs?repo=try&revision=254573d6464e
Attachment #8715278 -
Attachment is obsolete: true
Attachment #8723522 -
Flags: review?(bugs)
Comment 118•9 years ago
|
||
Comment on attachment 8723522 [details] [diff] [review] off_tests_ver20.diff Please make sure this passes the test on all the platforms. Run the tests couple of times in tryserver before asking for check-in.
Attachment #8723522 -
Flags: review?(bugs) → review+
Assignee | ||
Comment 119•9 years ago
|
||
(In reply to Olli Pettay [:smaug] (HIGH REVIEW LOAD) from comment #118) > Please make sure this passes the test on all the platforms. Run the tests > couple of times in tryserver before asking for check-in. I checked all failures every time running on try-server. And all times I cannot see any issues. Issues happens only after checkin procedure. Maybe try-server has different environment?
Assignee | ||
Comment 120•9 years ago
|
||
So, if there are no objections, I put checkin-needed flag. Let's try it again.
Keywords: checkin-needed
Comment 121•9 years ago
|
||
Did you trigger the tests several times on tryserver?
Assignee | ||
Comment 122•9 years ago
|
||
(In reply to Olli Pettay [:smaug] from comment #121) > Did you trigger the tests several times on tryserver? Yes. As I wrote earlier all latest try-server tests (with pointer events) showed good results. One more time: https://treeherder.mozilla.org/#/jobs?repo=try&revision=fb03b0cd37c0
Comment 123•9 years ago
|
||
This doesn't apply to mozilla-central. Please rebase.
Flags: needinfo?(alessarik)
Keywords: checkin-needed
Assignee | ||
Comment 124•9 years ago
|
||
(In reply to Ryan VanderMeulen [:RyanVM] from comment #123) > This doesn't apply to mozilla-central. Please rebase. Checked on 2016-04-19 night. All patches were applied ok. Right sequence: testing_feauters_ver4.diff - original_official_tests_ver2.diff - off_tests_ver20.diff
Flags: needinfo?(ryanvm)
Keywords: checkin-needed
Comment 125•9 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/efd7dcae14fa https://hg.mozilla.org/integration/mozilla-inbound/rev/83ee3ac3f36a https://hg.mozilla.org/integration/mozilla-inbound/rev/5669557c3f71
Keywords: checkin-needed
Comment 126•9 years ago
|
||
thanks maksim for the checkin instructions, this helped landing this.
Flags: needinfo?(ryanvm)
Comment 127•9 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/efd7dcae14fa https://hg.mozilla.org/mozilla-central/rev/83ee3ac3f36a https://hg.mozilla.org/mozilla-central/rev/5669557c3f71
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
status-firefox48:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla48
Assignee | ||
Comment 128•9 years ago
|
||
Perfectly! Congratulations! 2-year-long bug. I hope it will help to protect pointer-events from regressions!
Assignee | ||
Updated•9 years ago
|
You need to log in
before you can comment on or make changes to this bug.
Description
•