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

[GLib] Open test results in default browser if MiniBrowser fails #23177

Merged

Conversation

TingPing
Copy link
Contributor

@TingPing TingPing commented Jan 24, 2024

5a1edfb

[GLib] Open test results in default browser if MiniBrowser fails
https://bugs.webkit.org/show_bug.cgi?id=268015

Reviewed by Adrian Perez de Castro.

If something goes wrong, which happens during development, it is
useful to see results in a different browser rather than nothing.

* Tools/Scripts/webkitpy/port/gtk.py:
(GtkPort.show_results_html_file):

Canonical link: https://commits.webkit.org/273490@main

c076b70

Misc iOS, tvOS & watchOS macOS Linux Windows
✅ 🧪 style ✅ 🛠 ios ✅ 🛠 mac ✅ 🛠 wpe ✅ 🛠 wincairo
✅ 🧪 bindings ✅ 🛠 ios-sim ✅ 🛠 mac-AS-debug ✅ 🧪 wpe-wk2
✅ 🧪 webkitperl ✅ 🧪 ios-wk2 ✅ 🧪 api-mac ✅ 🧪 api-wpe
❌ 🧪 webkitpy ✅ 🧪 ios-wk2-wpt ✅ 🧪 mac-wk1 ✅ 🛠 gtk
✅ 🧪 api-ios ✅ 🧪 mac-wk2 ✅ 🧪 gtk-wk2
✅ 🛠 tv ✅ 🧪 mac-AS-debug-wk2 ✅ 🧪 api-gtk
✅ 🛠 tv-sim
🛠 🧪 merge ✅ 🛠 watch
✅ 🛠 🧪 unsafe-merge ✅ 🛠 watch-sim

@TingPing TingPing self-assigned this Jan 24, 2024
@TingPing TingPing added the WebKitGTK Bugs related to the Gtk API layer. label Jan 24, 2024
@TingPing TingPing requested review from a team and removed request for JonWBedard January 24, 2024 18:51
@TingPing
Copy link
Contributor Author

Personally I'd prefer it always ran in my default, but it was changed to do this not that long ago.

Copy link
Contributor

@aperezdc aperezdc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wouldn't it be better to try webbrowser.open_new_tab() before the gio tool? In that case we could have that as common fallback for all the ports.

@TingPing
Copy link
Contributor Author

webbrowser.open doesn't work on my system 🤷. gio correctly does what it should on Linux at least.

@TingPing
Copy link
Contributor Author

xdg-open is broken in my container, but that is on me I guess. I can switch it to webbrowser.

@aperezdc
Copy link
Contributor

webbrowser.open doesn't work on my system 🤷. gio correctly does what it should on Linux at least.

That's amusing, the webbrowser module works just fine here. Anyway, I have nothing against using gio, so patch LGTM anyway.

@TingPing
Copy link
Contributor Author

All of the other ports do very port specific things for show_results_html_file so I think it would be glib specific anyway.

@TingPing TingPing force-pushed the pgriffis/open-results-in-browser branch from 4af85bc to c076b70 Compare January 24, 2024 19:30
@mcatanzaro
Copy link
Contributor

Personally I'd prefer it always ran in my default, but it was changed to do this not that long ago.

To change that, we'd first really need to figure out how to make local subresources work under flatpak. The test results page is going to be super messed up otherwise.

@TingPing
Copy link
Contributor Author

webkitpy failure is unrelated:

[2410/2441] webkitpy.port.server_process_unittest.TestServerProcess.serial_test_basic erred:
  Traceback (most recent call last):
    File "/Volumes/Data/worker/WebKitPy-Tests-EWS/build/Tools/Scripts/webkitpy/port/server_process_unittest.py", line 125, in serial_test_basic
      self.assertEqual(line.strip(), b"stdout")
  AttributeError: 'NoneType' object has no attribute 'strip'

@TingPing TingPing added merge-queue Applied to send a pull request to merge-queue unsafe-merge-queue Applied to send a pull request to merge-queue, but skip building and testing and removed merge-queue Applied to send a pull request to merge-queue labels Jan 25, 2024
https://bugs.webkit.org/show_bug.cgi?id=268015

Reviewed by Adrian Perez de Castro.

If something goes wrong, which happens during development, it is
useful to see results in a different browser rather than nothing.

* Tools/Scripts/webkitpy/port/gtk.py:
(GtkPort.show_results_html_file):

Canonical link: https://commits.webkit.org/273490@main
@webkit-commit-queue
Copy link
Collaborator

Committed 273490@main (5a1edfb): https://commits.webkit.org/273490@main

Reviewed commits have been landed. Closing PR #23177 and removing active labels.

@webkit-commit-queue webkit-commit-queue merged commit 5a1edfb into WebKit:main Jan 25, 2024
@webkit-commit-queue webkit-commit-queue removed the unsafe-merge-queue Applied to send a pull request to merge-queue, but skip building and testing label Jan 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
WebKitGTK Bugs related to the Gtk API layer.
Projects
None yet
5 participants