Skip to content

[pkg:test] Fix binary location of Firefox on Mac #2274

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

Closed
wants to merge 1 commit into from
Closed

Conversation

kevmoo
Copy link
Member

@kevmoo kevmoo commented Aug 29, 2024

No description provided.

@kevmoo kevmoo requested a review from a team as a code owner August 29, 2024 20:04
Copy link

PR Health

Changelog Entry ✔️
Package Changed Files

Changes to files need to be accounted for in their respective changelogs.

Package publish validation ✔️
Package Version Status
package:checks 0.3.1-wip WIP (no publish necessary)
package:test 1.25.9-wip WIP (no publish necessary)
package:test_api 0.7.4-wip WIP (no publish necessary)
package:test_core 0.6.6-wip WIP (no publish necessary)

Documentation at https://github.com/dart-lang/ecosystem/wiki/Publishing-automation.

@kevmoo
Copy link
Member Author

kevmoo commented Aug 29, 2024

@jakemac53 – thoughts on this?

@jakemac53
Copy link
Contributor

@jakemac53 – thoughts on this?

I have no context so I have no thoughts :). Did it move? Does this location work on older versions? Maybe we need a fallback to the old location etc...

@kevmoo
Copy link
Member Author

kevmoo commented Aug 29, 2024

it DID move @jakemac53

FF tests were failing locally until I made this fix.
I guess I can open an issue that we actually test FF + Mac in CI, right?

@jakemac53
Copy link
Contributor

I guess I can open an issue that we actually test FF + Mac in CI, right?

I don't know if anybody has the bandwidth to actually look into this, there might be an open issue already.

@jakemac53
Copy link
Contributor

jakemac53 commented Aug 30, 2024

See also #2194 and #2195 ... the tldr; here is that it seems like adding support for multiple paths was never agreed upon and no matter what we do it doesn't work for some people (unless we add that support).

But, maybe this change has existed for long enough that it is time to make the switch, and highlight the workaround for anybody on older versions (using a custom path for Firefox via dart_test.yaml).

cc @natebosch thoughts?

@natebosch
Copy link
Member

#2276 gives a try at making this non-breaking and keeping the scope of the change as small as possible.

I haven't tested it yet, but I can if we think we want to land it.
I'm 50/50 on whether I want to try to land something minimal like that, or to just make the switch assuming more users will benefit than suffer, and document the workaround.

Do we have any details on when it moved, or how many users are likely to have either path?

@kevmoo
Copy link
Member Author

kevmoo commented Sep 10, 2024

Happy to run with #2276 if that's good.

Want me to close this?

@kevmoo kevmoo closed this Sep 10, 2024
@kevmoo kevmoo deleted the firefox_mac branch September 10, 2024 17:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants