File associations by davidfokkema · Pull Request #2284 · beeware/briefcase

davidfokkema · 2025-05-13T20:15:15Z

This PR adds file associations to macOS apps. The support in briefcase.toml is already there and the feature is documented. It wasn't yet generalised for non-package file types.

Refs #1706 (the issue)
Refs beeware/briefcase-macOS-app-template#72 (the PR for the macOS app template)
Refs beeware/briefcase-macOS-Xcode-template#82 (the PR for the macOS Xcode template)

This PR completes the work in the briefcase CLI tool.

I tested with https://github.com/davidfokkema/helloworld. This repository may be removed when the PR is merged.

PR Checklist:

All new features have been tested
All new features have been documented
I have read the CONTRIBUTING.md file
I will abide by the code of conduct

freakboy3742

The mechanics of this look good; a couple of minor comments, and obviously the testing issues still need to be addressed.

src/briefcase/config.py

src/briefcase/platforms/macOS/utils.py

src/briefcase/config.py

Co-authored-by: Russell Keith-Magee <russell@keith-magee.com>

…riefcase into file-associations

davidfokkema · 2025-05-14T10:49:52Z

I have two code coverage issues of which I'm not sure how to proceed. The aforementioned exception if the plist file is not in the correct location works fine in manual testing, but is not a part of the tests since the location is hardcoded in the file. So no 100% coverage. Should I change the path to a global variable, alter that in testing to check for the exception? Or should I skip the coverage for that branch?

Second issue is the if-statement in document validation where it checks the platform. If it is not macOS, it should not do the mime type to UTI conversion. I never test on macOS that it is not macOS, so coverage.py complains.

freakboy3742 · 2025-05-14T11:18:23Z

I have two code coverage issues of which I'm not sure how to proceed. The aforementioned exception if the plist file is not in the correct location works fine in manual testing, but is not a part of the tests since the location is hardcoded in the file. So no 100% coverage. Should I change the path to a global variable, alter that in testing to check for the exception? Or should I skip the coverage for that branch?

There's a couple of possible approaches I can see - but your idea of putting the name of the file as a constant and then mocking/monkeypatching that constant for the purposes of the "file doesn't exist" test sounds good to me.

Second issue is the if-statement in document validation where it checks the platform. If it is not macOS, it should not do the mime type to UTI conversion. I never test on macOS that it is not macOS, so coverage.py complains.

What we really need here is a conditional no-branch... but I don't think our conditional coverage handling can do that. The best fix I can see there is to put in a no-op else: pass, and mark that else as no-cover-if-is-macos.

davidfokkema · 2025-05-14T21:00:46Z

There's currently one failed unit test because of a timeout which I don't really understand since it seems that the failing test mocks a download, so... should never time out? Not sure. Restarting the GitHub action will probably solve this for now.

freakboy3742 · 2025-05-15T10:38:14Z

There's currently one failed unit test because of a timeout which I don't really understand since it seems that the failing test mocks a download, so... should never time out? Not sure. Restarting the GitHub action will probably solve this for now.

Hrm... I don't think it's doing a download, but it may be probing a live URL, which would make it sensitive to a temporary network outage. I haven't seen that failure before, though - I've kicked off the test again, and it looks like it's passed this time.

Specifying multiple values in a LSItemContentTypes key is only valid when multiple document types are manually grouped together in the Info.plist file. For Briefcase apps, document types are always separately declared in the configuration file, so only a single value must be provided. Because the key name is plural and is an array in the Info.plist file, a list *is* accepted, but must have only one element.

Since the code to pass coverage was ugly and should be removed in any case, I'm now passing pre-commit. I'm really not sure why coverage.py keeps complaining the branch is never reached, when it clearly _is_.

davidfokkema · 2025-05-18T15:03:41Z

A new issue has arisen: coverage. Coverage.py keeps complaining one branch of an if-statement is never reached in the tests, but I know for a fact that it is reached. If I include an else: 1 / 0 then the code crashes, if I include an else: foo = 1 then the code does not crash and coverage.py is happy, but apart from the ugly hack the pre-commit step fails because foo is never used. I could fix that with another line foo += 1 or something like that but that is even uglier. I removed it, and maybe someone else can figure out why coverage.py complains. I believe passing a string to LSItemContentTypes takes the 'missing' branch, and that is tested in the test_document_type_macOS_config_with_list_of_single_content_type test in tests/config/test_document_type_config.py.

mhsmith · 2025-05-21T20:41:54Z

The coverage error is src/briefcase/config.py 168->exit. This means that the if statement on line 168 has been executed, but it always evaluates to true. So either the if is redundant and can be removed, or you need to add a test for the case where the if evaluates to false.

davidfokkema · 2025-05-21T20:53:30Z

The coverage error is src/briefcase/config.py 168->exit. This means that the if statement on line 168 has been executed, but it always evaluates to true. So either the if is redundant and can be removed, or you need to add a test for the case where the if evaluates to false.

I don't have much experience with coverage.py, so thanks for taking a look! The if-statement checks whether the supplied content type is a list, and the 'else' means 'no list'. I believe I test that here:

briefcase/tests/config/test_document_type_config.py

Line 236 in 5b28ded

valid_document.setdefault("macOS", {})["LSItemContentTypes"] = "com.adobe.pdf"

. If I add an else: 1 / 0 statement that test crashes. So either I'm doing something wrong, I'm missing something obvious, or coverage.py is a bit confused.

mhsmith · 2025-05-21T21:36:59Z

Are you sure you're adding the else at the correct indentation level? There are several nested ifs here.

...but I don't like the fix.

davidfokkema · 2025-05-22T10:25:22Z

src/briefcase/config.py

+        else:
+            True is True


These two lines seem to fix the coverage check, but obviously they don't do anything.

@mhsmith, but this fails the ruff pre-commit check with https://docs.astral.sh/ruff/rules/useless-comparison/. Of course. I don't know what the best course of action is.

We had a similar mysterious coverage failure in #2308, but I don't think we ever worked out the cause.

Ah, I see the commit, thanks. I've now pushed a no-op which hopefully satisfies both the coverage checker and the pre-commit check and will leave that in for now. If it's a bug in coverage.py, this will hopefully be fixed in the future and this can go out. Not sure if I have enough time and inclination to create an MRE from this to file as an issue on coverage.py.

freakboy3742

This is getting pretty close; a couple of minor tweaks and suggestions inline.

changes/2284.feature.rst

src/briefcase/config.py

tests/config/test_document_type_config.py

freakboy3742 · 2025-05-23T09:43:27Z

tests/config/test_document_type_config.py

+def test_mime_type_to_uti_with_nonexisting_coretypes_file(monkeypatch):
+    """Test that mime_type_to_UTI returns None if the coretypes file doesn't exist."""
+    monkeypatch.setattr(utils, "CORETYPES_PATH", "/does/not/exist")
+    assert utils.mime_type_to_UTI("application/pdf") is None


For consistency, this should either (a) be test at the validate_document_type_config() level (i.e., a reproduction of the actual application/pdf case, but with the "couldn't find UTI data" response, or a separate test in the macOS module that does a couple of superficial direct checks of mime_type_to_UTI().

I'm not sure I follow. Could you elaborate, please?

All the other tests in this file are based on invoking validate_document_type_config(); but this test is invoking utils.mime_type_to_UTI. The test suite is broadly organised by test function; a test of mime_type_to_UTI should either be in a test file for that method; or the entry point for for this specific test should be modified to use validate_document_type_config() (i.e., testing the same thing, but doing so by setting up test conditions that will result in performing a mime type lookup).

To that end - this test file should also be called test_validate_document_type_config.

I've reshuffled the tests. Renamed test_document_type_config.py to test_validate_document_type_config.py and split out test_is_uti_core_type.py and test_mime_type_to_uti.py, and the first test file no longer depends on the briefcase.platforms.macOS.utils module.

tests/config/test_document_type_config.py

Co-authored-by: Russell Keith-Magee <russell@keith-magee.com>

davidfokkema · 2025-05-25T08:57:24Z

(At least) one failing test needs a rerun:

Unable to download RCEdit; is your computer offline?

davidfokkema · 2025-05-25T19:45:29Z

Added some documentation to describe the document types, especially the new macOS-specific support. Probably a work in progress, but it is a first draft.

freakboy3742 · 2025-05-28T05:57:16Z

FYI - I wanted to let you know that I'm not ignoring this PR; I'm currently buried in post-PyCon US backlog. I'm hoping to get to this in a day or two.

davidfokkema · 2025-05-28T07:46:37Z

Sure, no worries!

freakboy3742

First off - apologies for the delay in reviewing this. "A couple of days" turned out to be overly optimistic.

But - this is awesome stuff. I've done an update pass on the documentation, and tweaked some tests (mostly as penance for being so slow on the review); but I think this is good to go. Thanks for all your work on this - Briefcase's doctype handling is a lot more robust as a result of your efforts.

freakboy3742 · 2025-05-27T05:07:00Z

docs/reference/configuration.rst

+``application/x-myapp-data``. This is not a valid MIME type, and should not be
+used for production applications. It is only used for testing purposes, to allow


AFAIK, this is incorrect. The application/x- prefix is defined by RFC 2046 for "private and experimental" MIME types - it exists specifically for expansion by end users without needing to go through formal IANA registration - so it is valid.

freakboy3742 · 2025-06-13T07:13:19Z

docs/reference/platforms/macOS/index.rst

+Background
+~~~~~~~~~~
+
+First, macOS document types are defined in the ``Info.plist`` file, and the


These docs are great from the perspective of macOS internals, but they're not as practical for a Briefcase user. Reading these docs, my immediate question was "ok... but what do I have to do?".

And - for most users, the answer is "nothing". I've pushed an update to these docs that changes the focus to more of a "how does Briefcase use these settings", rather than trying to explain Apple's Info.plist scheme.

freakboy3742 · 2025-06-13T07:14:03Z

src/briefcase/config.py

+        elif isinstance(content_types, str):
+            # If the content type is a string, convert it to a list
+            macOS["LSItemContentTypes"] = [content_types]
+        else:


These no-op branches can be cleaned up if the content type handling is performed before the UTI handling - I'll push an update to do this.

freakboy3742 · 2025-06-13T07:14:32Z

tests/config/test_AppConfig.py

                "extension": "doc",
                "description": "A document",
                "url": "https://testurl.com",
+                "mime_type": "text/x.my-doc-type",


This isn't a valid doctype - we should be using application/x-my-doc-type.

freakboy3742 · 2025-06-13T07:14:43Z

tests/config/test_is_uti_core_type.py

+
+
+@pytest.mark.skipif(sys.platform != "darwin", reason="Test runs only on macOS")
+def test_is_uti_core_type():


This test can be parameterised.

freakboy3742 · 2025-06-13T07:14:55Z

tests/config/test_mime_type_to_uti.py

+
+
+@pytest.mark.skipif(sys.platform != "darwin", reason="Test runs only on macOS")
+def test_mime_type_to_uti():


This test can be parameterized.

davidfokkema · 2025-06-13T11:58:10Z

No need to apologise, I first started working on this back in March, 2024, but did not have time to finish this for many, many months. Thank you for your encouragement, guidance, and careful reviews throughout the process! I've looked through your latest changes; I like them. Thanks for merging!

davidfokkema added 5 commits May 11, 2025 11:05

WIP: determining correct UTI from mime type

8ece0d5

Set plist default values from UTI

4264d3b

Remove print statements

4b01f9d

Merge remote-tracking branch 'upstream/main' into file-associations

4625cbe

Replace match-statement with if-statement

1327b07

davidfokkema mentioned this pull request May 13, 2025

Generalised file associations beeware/briefcase-macOS-app-template#72

Merged

4 tasks

Added change note

f82f484

davidfokkema mentioned this pull request May 13, 2025

Add file associations to Windows and macOS installers #1706

Closed

Add macOS doctype default variables to test

5c8faed

freakboy3742 reviewed May 14, 2025

View reviewed changes

src/briefcase/config.py Outdated Show resolved Hide resolved

src/briefcase/platforms/macOS/utils.py Outdated Show resolved Hide resolved

src/briefcase/config.py Outdated Show resolved Hide resolved

davidfokkema and others added 4 commits May 14, 2025 12:40

Added tests and no coverage pragmas

c70ebf6

Split out extraction of MIME type and rename variable

03f6b55

Co-authored-by: Russell Keith-Magee <russell@keith-magee.com>

If plist file is moved in OS update, catch exception

b61fbe6

Merge branch 'file-associations' of https://github.com/davidfokkema/b…

904b12b

…riefcase into file-associations

Add descriptions of tests

b493106

davidfokkema added 2 commits May 14, 2025 22:17

Complete code coverage

6c4b3c2

Run macOS-specific tests only on macOS

9323eba

davidfokkema added 6 commits May 16, 2025 17:20

Built-in types should have attribute set to True

a7f5620

Renamed builtin_type -> is_core_type

a3c8efd

Fix failing test

07c1565

Add tests to complete coverage

41cdc7a

Pass pre-commit, but fail coverage

31f8342

Since the code to pass coverage was ugly and should be removed in any case, I'm now passing pre-commit. I'm really not sure why coverage.py keeps complaining the branch is never reached, when it clearly _is_.

Remove old test code

5b28ded

Fix coverage

a810a4c

...but I don't like the fix.

davidfokkema commented May 22, 2025

View reviewed changes

No-op to satisfy coverage checker

fdbf75d

freakboy3742 requested changes May 23, 2025

View reviewed changes

davidfokkema and others added 5 commits May 23, 2025 12:40

Update changes/2284.feature.rst

7412a90

Co-authored-by: Russell Keith-Magee <russell@keith-magee.com>

More test cases and improved LSItemContentTypes handling

2a3b44d

Add test to fix coverage

1533af9

Skip coverage if not macOS

46d2cb6

Reshuffled document type tests

fd48660

davidfokkema added 2 commits May 25, 2025 21:39

Document the new document type support

06c12d5

Rewrap docs

df5909d

Trim trailing whitespace

8203397

freakboy3742 added 6 commits June 13, 2025 14:57

Simplify documentation to focus on the Briefcase use cases.

7d96377

Small cleanup of logic in macOS doctype processing.

7131e6e

Cleanup of test cases.

bae3210

Tweak release note.

d51b53b

Merge branch 'main' into file-associations

9374c63

Restore some additional reading links on macOS document types.

63f594f

freakboy3742 approved these changes Jun 13, 2025

View reviewed changes

freakboy3742 mentioned this pull request Jun 13, 2025

Generalised file associations beeware/briefcase-macOS-Xcode-template#82

Merged

4 tasks

Correct spelling.

21ee20d

freakboy3742 merged commit e80edff into beeware:main Jun 13, 2025
57 checks passed

		``application/x-myapp-data``. This is not a valid MIME type, and should not be
		used for production applications. It is only used for testing purposes, to allow



		@pytest.mark.skipif(sys.platform != "darwin", reason="Test runs only on macOS")
		def test_is_uti_core_type():



		@pytest.mark.skipif(sys.platform != "darwin", reason="Test runs only on macOS")
		def test_mime_type_to_uti():

Uh oh!

Conversation

davidfokkema commented May 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Checklist:

Uh oh!

freakboy3742 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

davidfokkema commented May 14, 2025

Uh oh!

freakboy3742 commented May 14, 2025

Uh oh!

davidfokkema commented May 14, 2025

Uh oh!

freakboy3742 commented May 15, 2025

Uh oh!

davidfokkema commented May 18, 2025

Uh oh!

mhsmith commented May 21, 2025

Uh oh!

davidfokkema commented May 21, 2025

Uh oh!

mhsmith commented May 21, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

freakboy3742 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

davidfokkema commented May 25, 2025

Uh oh!

davidfokkema commented May 25, 2025

Uh oh!

freakboy3742 commented May 28, 2025

Uh oh!

davidfokkema commented May 28, 2025

Uh oh!

freakboy3742 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

davidfokkema commented Jun 13, 2025

Uh oh!

Reviewers

Assignees

davidfokkema commented May 13, 2025 •

edited

Loading