New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 831847 link

Starred by 5 users

Issue metadata

Status: Fixed
Owner:
Closed: Jun 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Windows
Pri: 1
Type: Bug-Regression

Blocked on:
issue 839183
issue 810437



Sign in to add a comment

Re-enable webkit_layout_tests on win7_chromium_rel_ng

Project Member Reported by hzl@chromium.org, Apr 11 2018

Issue description

Components: Tests
Labels: -Pri-2 OS-Windows Pri-1
Summary: Re-enable webkit_layout_tests on win7_chromium_rel_ng (was: Re-enable WebKit Win 7)
Owner: martiniss@chromium.org
* external/wpt/dom/historical.html
* external/wpt/encoding/legacy-mb-japanese/iso-2022-jp/iso2022jp-encode-form-csiso2022jp.html
* external/wpt/encoding/legacy-mb-tchinese/big5/big5-enc-ascii.html
* external/wpt/websockets/Create-protocols-repeated-case-insensitive.any.html
* virtual/layout_ng_experimental/fast/multicol/border-radius-clipped-layer-second-column.html
 

Are currently failing on the bot. I guess I'll just disable them? 

Also, the bot isn't uploading layout test results, which is bad and a bug and I should fix it.
Project Member

Comment 4 by bugdroid1@chromium.org, Apr 26 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/tools/build/+/40f838c667a15a593afee6526926f933438177c5

commit 40f838c667a15a593afee6526926f933438177c5
Author: Stephen Martinis <martiniss@chromium.org>
Date: Thu Apr 26 20:56:19 2018

Upload layout tests for experimental runs

TBR=jbudorick

Bug:  831847 
Change-Id: I99cb2e6dcc3eea14f49e13785cd19d4a48c32793
Reviewed-on: https://chromium-review.googlesource.com/1031174
Reviewed-by: Stephen Martinis <martiniss@chromium.org>
Commit-Queue: Stephen Martinis <martiniss@chromium.org>

[modify] https://crrev.com/40f838c667a15a593afee6526926f933438177c5/scripts/slave/recipe_modules/chromium_tests/steps.py

https://chromium-review.googlesource.com/c/infra/luci/recipes-py/+/1008166 was supposed to fix the infra failures, but didn't. I need to look into this more. 
Cc: martiniss@chromium.org iannucci@chromium.org
 Issue 837301  has been merged into this issue.
Status: Started (was: Assigned)
I'm looking to see how we can re-enable this.
Project Member

Comment 8 by bugdroid1@chromium.org, Apr 27 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/infra/luci/recipes-py/+/8bb8a8eddc83b7110083f1fef2aaafaf7bd25d90

commit 8bb8a8eddc83b7110083f1fef2aaafaf7bd25d90
Author: Stephen Martinis <martiniss@google.com>
Date: Fri Apr 27 19:15:46 2018

Handle long paths on Windows part 2

Followup to https://crrev.com/c/1008166. That seems to have worked, but there's
another code path which needs the prefix. Testing with led verified
this seems to have made it pass.

Bug:  831847 
Change-Id: I011a4bdbb0dc2dedd7ebb45635075448c49e7080
Reviewed-on: https://chromium-review.googlesource.com/1033581
Reviewed-by: John Budorick <jbudorick@chromium.org>
Reviewed-by: Marc-Antoine Ruel <maruel@chromium.org>
Commit-Queue: Stephen Martinis <martiniss@chromium.org>

[modify] https://crrev.com/8bb8a8eddc83b7110083f1fef2aaafaf7bd25d90/recipe_modules/raw_io/api.py

Project Member

Comment 9 by bugdroid1@chromium.org, Apr 27 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/109eed414a06aaae92ef600c28c71f2415a52085

commit 109eed414a06aaae92ef600c28c71f2415a52085
Author: Stephen Martinis <martiniss@chromium.org>
Date: Fri Apr 27 21:12:12 2018

Reland "Re-add layout tests to win7 bot"

This reverts commit aac2bcc130900561b460ddc60ae13031846076f9.

Reason for revert: Windows path issues hopefully fixed

Original change's description:
> Revert "Re-add layout tests to win7 bot"
>
> This reverts commit d7a83cd8efeaf932ae023f3d961b70bd3e0ab5b1.
>
> Reason for revert: bots are still infra failing
>
> Original change's description:
> > Re-add layout tests to win7 bot
> >
> > Bug:  831585 
> > Change-Id: I3436ca3abe39a648a150d703173715f69756e508
> > Reviewed-on: https://chromium-review.googlesource.com/1028009
> > Commit-Queue: Stephen Martinis <martiniss@chromium.org>
> > Reviewed-by: Dirk Pranke <dpranke@chromium.org>
> > Reviewed-by: John Budorick <jbudorick@chromium.org>
> > Cr-Commit-Position: refs/heads/master@{#553868}
>
> TBR=dpranke@chromium.org,martiniss@chromium.org,jbudorick@chromium.org
>
> Change-Id: I4d08d6e21a0ac4622d7a936ff92a5bd795d5af13
> No-Presubmit: true
> No-Tree-Checks: true
> No-Try: true
> Bug:  831585 
> Reviewed-on: https://chromium-review.googlesource.com/1031530
> Reviewed-by: Stephen Martinis <martiniss@chromium.org>
> Commit-Queue: Stephen Martinis <martiniss@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#554165}

TBR=dpranke@chromium.org,martiniss@chromium.org,jbudorick@chromium.org

Change-Id: I13aa35358d093b09b14125ffe62535ddc4c77ffd
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  831847 
Reviewed-on: https://chromium-review.googlesource.com/1033792
Commit-Queue: Stephen Martinis <martiniss@chromium.org>
Reviewed-by: Stephen Martinis <martiniss@chromium.org>
Cr-Commit-Position: refs/heads/master@{#554509}
[modify] https://crrev.com/109eed414a06aaae92ef600c28c71f2415a52085/testing/buildbot/chromium.win.json
[modify] https://crrev.com/109eed414a06aaae92ef600c28c71f2415a52085/testing/buildbot/test_suite_exceptions.pyl

Ok, CI build almost passed.

4 tests are failing. They look like they're actually failing? I'm leaning towards disabling them, I'm not sure what's wrong with them.
Cc: dpranke@chromium.org robertma@chromium.org
I tried to disable them, but ran into a presubmit error. https://chromium-review.googlesource.com/c/chromium/src/+/1034029 has more

The tests are currently running, but experimental, so aren't blocking CLs from landing at all.
Ok, tests are still broken. blink script stuff now is breaking because of long file names. 

From https://logs.chromium.org/v/?s=chromium%2Fbb%2Ftryserver.chromium.win%2Fwin7_chromium_rel_ng%2F157075%2F%2B%2Frecipes%2Fsteps%2Fwebkit_layout_tests__with_patch__experimental_%2F0%2Fstdout


Traceback (most recent call last):
  File "C:\b\c\b\win\src\third_party\blink\tools\merge_web_test_results.py", line 12, in <module>
    main(sys.argv[1:])
  File "C:\b\c\b\win\src\third_party\blink\tools\blinkpy\web_tests\merge_results.py", line 823, in main
    merger.merge(args.output_directory, args.input_directories)
  File "C:\b\c\b\win\src\third_party\blink\tools\blinkpy\web_tests\merge_results.py", line 514, in merge
    merge_func(out_path, to_merge)
  File "C:\b\c\b\win\src\third_party\blink\tools\blinkpy\web_tests\merge_results.py", line 287, in __call__
    self.filesystem.copyfile(to_merge[0], out_filename)
  File "C:\b\c\b\win\src\third_party\blink\tools\blinkpy\common\system\filesystem.py", line 84, in copyfile
    shutil.copyfile(source, destination)
  File "C:\b\depot_tools\win_tools-2_7_6_bin\python\bin\lib\shutil.py", line 82, in copyfile
    with open(src, 'rb') as fsrc:
IOError: [Errno 2] No such file or directory: 'c:\\users\\chrome~1\\appdata\\local\\temp\\tmpf8qo2a\\11\\layout-test-results\\virtual\\outofblink-cors\\external\\wpt\\referrer-policy\\strict-origin-when-cross-origin\\meta-referrer\\cross-origin\\http-https\\fetch-request\\upgrade-protocol.swap-origin-redirect.http-stderr.txt'
WARNING:root:merge_cmd had non-zero return code: 1
step returned non-zero exit code: 1

We need to do the same things we did to the recipe engine to blink scripts.

That's fairly non trivial, so disabling them in CQ for now.
We're not going to do the same thing to the blink scripts.

We're going to try to limit the filename paths instead. robertma@ will do that, I've filed https://crbug.com/839183 for that.

I'll try to make the swarming temp dire path shorter. I think it's doable?

Comment 15 by kbr@chromium.org, May 4 2018

Blocking: 839183
Cc: mar...@chromium.org
I thought maruel@ had a Python trampoline which lifted the Windows path length limitation, and that it was mainly Python (and not any other Chrome infrastructure or binaries) that was running into the limitation.

We should prefer that so that Windows behaves the same as other platforms.

It's https://cs.chromium.org/chromium/infra/luci/client/utils/fs.py
It's simple, it works great, but it has to be used everywhere consistently, which can be a challenge. For the Swarming bot itself it was more tractable.
That's what I was going to do with the blink scripts in https://chromium-review.googlesource.com/c/chromium/src/+/1038431. The people who knew that code better ended up not wanting to do that.
> We should prefer that so that Windows behaves the same as other platforms.

My preference here is that we enforce the max path (of 160 or 170 or whatever it needs to be) consistently via PRESUBMITs and tests, rather than try to work around it via things like fs.py. As maruel@ says, it has to be used consistently and I think it can be too difficult to do that, whereas writing presubmits should be straightforward and fairly bulletproof.
Blocking: -839183
Blockedon: 839183
Having looked at this more, perhaps the CL in #c17 is the way to go. I apologize for shooting it down potentially too quickly.
Blockedon: 810437
Apart from the path issue that I'm trying to work around and prevent (issue 839183), there are also a bunch of tests that are genuinely flaky, particularly so on Windows ( issue 810437  has some of them).
Project Member

Comment 23 by bugdroid1@chromium.org, May 24 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/ace37f02189e5fadde26690bcc248fba152739d5

commit ace37f02189e5fadde26690bcc248fba152739d5
Author: Robert Ma <robertma@chromium.org>
Date: Thu May 24 21:00:09 2018

[blinkpy] Add UNC magic prefix to Windows paths

Based on https://crrev.com/c/1038431

Bug:  831847 
Change-Id: Id3ce9733954f9c97f5bb67ebaa8e3c9c6d436cb9
Reviewed-on: https://chromium-review.googlesource.com/1064805
Reviewed-by: Dirk Pranke <dpranke@chromium.org>
Reviewed-by: Stephen Martinis <martiniss@chromium.org>
Reviewed-by: Quinten Yearsley <qyearsley@chromium.org>
Commit-Queue: Robert Ma <robertma@chromium.org>
Cr-Commit-Position: refs/heads/master@{#561625}
[modify] https://crrev.com/ace37f02189e5fadde26690bcc248fba152739d5/third_party/blink/tools/blinkpy/common/system/filesystem.py
[modify] https://crrev.com/ace37f02189e5fadde26690bcc248fba152739d5/third_party/blink/tools/blinkpy/common/system/filesystem_unittest.py

I looked at the last ~50 builds, and spot checked layout tests. There are some failures, but none of the failures are consistent. And I don't think anything is filename length related. I'm going to re-enable the suite, and warn sheriffs that it might be flaky.
Project Member

Comment 25 by bugdroid1@chromium.org, Jun 6 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/5a1b8799d61909fbb11b172c305dbb544e891040

commit 5a1b8799d61909fbb11b172c305dbb544e891040
Author: Stephen Martinis <martiniss@chromium.org>
Date: Wed Jun 06 20:52:32 2018

Promote layout tests to prod on Win7

The tests are passing on CI now, so make it block the CQ.

NOTRY=true

Bug:  831847 
Change-Id: I9767bb792b99cf3a47975062f6820c1fc82f8f6a
Reviewed-on: https://chromium-review.googlesource.com/1035794
Commit-Queue: Stephen Martinis <martiniss@chromium.org>
Reviewed-by: Dirk Pranke <dpranke@chromium.org>
Reviewed-by: John Budorick <jbudorick@chromium.org>
Cr-Commit-Position: refs/heads/master@{#565030}
[modify] https://crrev.com/5a1b8799d61909fbb11b172c305dbb544e891040/testing/buildbot/chromium.win.json
[modify] https://crrev.com/5a1b8799d61909fbb11b172c305dbb544e891040/testing/buildbot/test_suite_exceptions.pyl

Status: Fixed (was: Started)

Sign in to add a comment