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

[Azure Pipelines] Upgrade macOS VMs to Mojave #17219

Closed
wants to merge 2 commits into from

Conversation

@foolip
Copy link
Contributor

foolip commented Jun 6, 2019

No description provided.

@wpt-pr-bot wpt-pr-bot added the infra label Jun 6, 2019
@wpt-pr-bot wpt-pr-bot requested a review from jgraham Jun 6, 2019
@foolip

This comment has been minimized.

Copy link
Contributor Author

foolip commented Jun 6, 2019

I opened #17217 to be able to see if any failures are pre-existing. I'll also push that commit to this branch to this PR to see if everything passes. Additionally, I'll trigger a full run of the same type as runs every 6 hours.

@foolip

This comment has been minimized.

Copy link
Contributor Author

foolip commented Jun 6, 2019

@foolip

This comment has been minimized.

Copy link
Contributor Author

foolip commented Jun 6, 2019

https://wpt.fyi/results/?diff&filter=ADC&run_id=266520006&run_id=242570013 compares results from 10.13 and 10.14. Looks OK except for a huge regression in css-grid results. The exact same commit wasn't tested, so I'll try again after the next Safari TP run on master.

@foolip foolip force-pushed the foolip/mojave branch from d990605 to 8bb9735 Jun 7, 2019
@gsnedders

This comment has been minimized.

Copy link
Contributor

gsnedders commented Jun 7, 2019

FWIW, those grid results match what I get locally in STP 84.

@foolip

This comment has been minimized.

Copy link
Contributor Author

foolip commented Jun 7, 2019

New build on top of commit 38bf3be in progress:
https://dev.azure.com/web-platform-tests/wpt/_build/results?buildId=18475

Sent #17225 to be able to do the same for stable.

@foolip foolip force-pushed the foolip/mojave branch from fb0fa05 to 94f4fd3 Jun 11, 2019
@foolip

This comment has been minimized.

Copy link
Contributor Author

foolip commented Jun 11, 2019

@foolip

This comment has been minimized.

Copy link
Contributor Author

foolip commented Jun 11, 2019

@foolip

This comment has been minimized.

Copy link
Contributor Author

foolip commented Jun 11, 2019

Aha! For both Safaris /infrastructure/assumptions/ahem.html is failing because Ahem appears to not be loaded. This is also the cause of some other failures such as /css/CSS2/text/letter-spacing-004.xht.

Rather than debugging system font installation on Mojave I'll wait for web-platform-tests/rfcs#22 to be resolved, we're pinned to Safari TP 82 anyway so there's no urgency to upgrade.

@gsnedders

This comment has been minimized.

Copy link
Contributor

gsnedders commented Jun 11, 2019

Pretty sure 10.14 is the point at which they stopped loading non-system fonts (by default?).

com.apple.Safari.ContentPageGroupIdentifier.WebKit2ShouldAllowUserInstalledFonts is the pref to configure it, but I think changing that requires disabling the sandboxing?

@foolip

This comment has been minimized.

Copy link
Contributor Author

foolip commented Jun 11, 2019

Oh, I didn't know that changes was coming / has come. Good thing that we went ahead with the Ahem change then! I won't try the setting to get the old behavior, @LukeZielinski's work on Ahem will probably resolve this in a matter of weeks anyway.

@foolip foolip force-pushed the foolip/mojave branch from 94f4fd3 to 04de8f6 Jun 12, 2019
@foolip

This comment has been minimized.

Copy link
Contributor Author

foolip commented Jun 12, 2019

Since #17173 was merged yesterday I've rebased and triggered another full run of the Safaris to see if the differences are fewer now:
https://dev.azure.com/web-platform-tests/wpt/_build/results?buildId=18914

@foolip

This comment has been minimized.

Copy link
Contributor Author

foolip commented Jun 12, 2019

@gsnedders

This comment has been minimized.

Copy link
Contributor

gsnedders commented Jun 12, 2019

A bunch of the regressions in CSS are just more things needed Ahem

@foolip

This comment has been minimized.

Copy link
Contributor Author

foolip commented Jun 12, 2019

It looks like "all tests (Safari Technology Preview) 1" in https://dev.azure.com/web-platform-tests/wpt/_build/results?buildId=18914 silently failed to run all of the tests it should have run. Comparing it to https://dev.azure.com/web-platform-tests/wpt/_build/results?buildId=18874 where the last test was in /xhr/, that job stopped at /payment-request/payment-is-showing.https.html:

  ▶ ERROR [expected OK] /payment-request/payment-is-showing.https.html
  └   → Traceback (most recent call last):
  File "/Users/vsts/agent/2.152.1/work/1/s/tools/wptrunner/wptrunner/executors/executorwebdriver.py", line 282, in _run
    self.result = True, self.func(self.protocol, self.url, self.timeout)
  File "/Users/vsts/agent/2.152.1/work/1/s/tools/wptrunner/wptrunner/executors/executorwebdriver.py", line 373, in do_testharness
    done, rv = handler(result)
  File "/Users/vsts/agent/2.152.1/work/1/s/tools/wptrunner/wptrunner/executors/base.py", line 604, in __call__
    return callback(url, payload)
  File "/Users/vsts/agent/2.152.1/work/1/s/tools/wptrunner/wptrunner/executors/base.py", line 622, in process_action
    self._send_message("complete", "error")
  File "/Users/vsts/agent/2.152.1/work/1/s/tools/wptrunner/wptrunner/executors/base.py", line 631, in _send_message
    self.protocol.testdriver.send_message(message_type, status, message=message)
  File "/Users/vsts/agent/2.152.1/work/1/s/tools/wptrunner/wptrunner/executors/executorwebdriver.py", line 189, in send_message
    self.webdriver.execute_script("window.postMessage(%s, '*')" % json.dumps(obj))
  File "/Users/vsts/agent/2.152.1/work/1/s/tools/webdriver/webdriver/client.py", line 20, in inner
    return func(self, *args, **kwargs)
  File "/Users/vsts/agent/2.152.1/work/1/s/tools/webdriver/webdriver/client.py", line 631, in execute_script
    return self.send_session_command("POST", "execute/sync", body)
  File "/Users/vsts/agent/2.152.1/work/1/s/tools/webdriver/webdriver/client.py", line 508, in send_session_command
    return self.send_command(method, url, body)
  File "/Users/vsts/agent/2.152.1/work/1/s/tools/webdriver/webdriver/client.py", line 472, in send_command
    raise err
InvalidSessionIdException: invalid session id (404): 



##[section]Finishing: Run tests

@gsnedders is "invalid session id" something from SafariDriver? Is this another case of #13073?

@gsnedders

This comment has been minimized.

Copy link
Contributor

gsnedders commented Jun 12, 2019

@foolip that should be recoverable; the irrecoverable error should be when we repeatedly fail to restart.

@gsnedders

This comment has been minimized.

Copy link
Contributor

gsnedders commented Jun 12, 2019

(Needs a rebase to pass CI, due to #17286. Sorry!)

@foolip

This comment has been minimized.

Copy link
Contributor Author

foolip commented Jun 12, 2019

@gsnedders is there a bug for making this case restart properly? We could potentially be getting bad runs already because of this.

@gsnedders

This comment has been minimized.

Copy link
Contributor

gsnedders commented Jun 13, 2019

@foolip we should already handle this; I think we really need logs with debug info in them to figure out what's going on (i.e., --log-mach - --log-mach-level debug).

foolip added a commit that referenced this pull request Aug 16, 2019
In #17219 (comment)
a number of tests were found to depend on document.fonts.ready in this
way, and it turns out it doesn't work in Safari with web fonts. Add a
test to surface this problem more clearly.
@foolip

This comment has been minimized.

Copy link
Contributor Author

foolip commented Aug 16, 2019

I've sent a minimized test in #18489. However, I think it'll pass in Safari on 10.13 where we still have the system font installed.

@foolip

This comment has been minimized.

Copy link
Contributor Author

foolip commented Aug 16, 2019

I think https://bugs.webkit.org/show_bug.cgi?id=174030 is the right WebKit bug for this, commented there.

foolip added a commit that referenced this pull request Aug 16, 2019
In #17219 (comment)
a number of tests were found to depend on document.fonts.ready in this
way, and it turns out it doesn't work in Safari with web fonts. Add a
test to surface this problem more clearly.
@foolip foolip force-pushed the foolip/mojave branch from 90079d8 to e733e58 Aug 17, 2019
moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this pull request Aug 23, 2019
…ng assumptions, a=testonly

Automatic update from web-platform-tests
Add a test for document.fonts.ready timing assumptions (#18489)

In web-platform-tests/wpt#17219 (comment)
a number of tests were found to depend on document.fonts.ready in this
way, and it turns out it doesn't work in Safari with web fonts. Add a
test to surface this problem more clearly.
--

wpt-commits: 29e72c566832e86e0180982400a1d77d4773e5d2
wpt-pr: 18489
xeonchen pushed a commit to xeonchen/gecko that referenced this pull request Aug 23, 2019
…ng assumptions, a=testonly

Automatic update from web-platform-tests
Add a test for document.fonts.ready timing assumptions (#18489)

In web-platform-tests/wpt#17219 (comment)
a number of tests were found to depend on document.fonts.ready in this
way, and it turns out it doesn't work in Safari with web fonts. Add a
test to surface this problem more clearly.
--

wpt-commits: 29e72c566832e86e0180982400a1d77d4773e5d2
wpt-pr: 18489
natechapin added a commit to natechapin/wpt that referenced this pull request Aug 23, 2019
…tests#18489)

In web-platform-tests#17219 (comment)
a number of tests were found to depend on document.fonts.ready in this
way, and it turns out it doesn't work in Safari with web fonts. Add a
test to surface this problem more clearly.
@foolip foolip force-pushed the foolip/mojave branch from e733e58 to 73441c6 Sep 9, 2019
@foolip

This comment has been minimized.

Copy link
Contributor Author

foolip commented Sep 11, 2019

https://bugs.webkit.org/show_bug.cgi?id=174030 has been fixed, but upgrading is still not possible. It's likely going to be necessary to upgrade both the OS and STP at the same time as I've attempted in #18925 but still isn't working. This PR won't be merged at any rate, so I'm closing it.

Will try to summarize the state of things in #17186.

@foolip foolip closed this Sep 11, 2019
Reliable Safari results automation moved this from To do to Done Sep 11, 2019
@foolip foolip deleted the foolip/mojave branch Sep 11, 2019
gecko-dev-updater pushed a commit to marco-c/gecko-dev-comments-removed that referenced this pull request Oct 4, 2019
…logging for full runs, a=testonly

Automatic update from web-platform-tests
[Azure Pipelines] Use more verbose TBPL logging for full runs (#17379)

This is to help diagnose problems if they occur, such as this failure
to run all of the tests:
web-platform-tests/wpt#17219 (comment)
--

wpt-commits: 3857b199e7ee7867550b268df05664e2ef055600
wpt-pr: 17379

UltraBlame original commit: a714f559961d6bdaecad4a5e2cf061f7fb5befe0
gecko-dev-updater pushed a commit to marco-c/gecko-dev-comments-removed that referenced this pull request Oct 4, 2019
…ng assumptions, a=testonly

Automatic update from web-platform-tests
Add a test for document.fonts.ready timing assumptions (#18489)

In web-platform-tests/wpt#17219 (comment)
a number of tests were found to depend on document.fonts.ready in this
way, and it turns out it doesn't work in Safari with web fonts. Add a
test to surface this problem more clearly.
--

wpt-commits: 29e72c566832e86e0180982400a1d77d4773e5d2
wpt-pr: 18489

UltraBlame original commit: cb4947af2d124fa06702d85a5fbcd6eb9dce5e52
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified that referenced this pull request Oct 4, 2019
…logging for full runs, a=testonly

Automatic update from web-platform-tests
[Azure Pipelines] Use more verbose TBPL logging for full runs (#17379)

This is to help diagnose problems if they occur, such as this failure
to run all of the tests:
web-platform-tests/wpt#17219 (comment)
--

wpt-commits: 3857b199e7ee7867550b268df05664e2ef055600
wpt-pr: 17379

UltraBlame original commit: a714f559961d6bdaecad4a5e2cf061f7fb5befe0
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified-and-comments-removed that referenced this pull request Oct 4, 2019
…logging for full runs, a=testonly

Automatic update from web-platform-tests
[Azure Pipelines] Use more verbose TBPL logging for full runs (#17379)

This is to help diagnose problems if they occur, such as this failure
to run all of the tests:
web-platform-tests/wpt#17219 (comment)
--

wpt-commits: 3857b199e7ee7867550b268df05664e2ef055600
wpt-pr: 17379

UltraBlame original commit: a714f559961d6bdaecad4a5e2cf061f7fb5befe0
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified that referenced this pull request Oct 4, 2019
…ng assumptions, a=testonly

Automatic update from web-platform-tests
Add a test for document.fonts.ready timing assumptions (#18489)

In web-platform-tests/wpt#17219 (comment)
a number of tests were found to depend on document.fonts.ready in this
way, and it turns out it doesn't work in Safari with web fonts. Add a
test to surface this problem more clearly.
--

wpt-commits: 29e72c566832e86e0180982400a1d77d4773e5d2
wpt-pr: 18489

UltraBlame original commit: cb4947af2d124fa06702d85a5fbcd6eb9dce5e52
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified-and-comments-removed that referenced this pull request Oct 4, 2019
…ng assumptions, a=testonly

Automatic update from web-platform-tests
Add a test for document.fonts.ready timing assumptions (#18489)

In web-platform-tests/wpt#17219 (comment)
a number of tests were found to depend on document.fonts.ready in this
way, and it turns out it doesn't work in Safari with web fonts. Add a
test to surface this problem more clearly.
--

wpt-commits: 29e72c566832e86e0180982400a1d77d4773e5d2
wpt-pr: 18489

UltraBlame original commit: cb4947af2d124fa06702d85a5fbcd6eb9dce5e52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
5 participants
You can’t perform that action at this time.