MSI: Integration tests by lrandersson · Pull Request #1133 · conda/constructor

lrandersson · 2025-12-09T20:37:18Z

Description

Opened this one separately, this branch and PR depends on #1084. No need to review this for now since the diff will look strange and is more for book-keeping since these changes were pulled out of PR 1084.

Checklist - did you ...

Add a file to the news directory (using the template) for the next release's release notes?
Add / update necessary tests?
Add / update outdated documentation?

marcoesters · 2025-12-17T14:14:52Z

.github/workflows/main.yml

+
+      - name: Install Briefcase (editable)
+        run: |
+          pip install -e briefcase


Is this still needed? Either way, this should only be installed for Windows.

You're right, I removed that

marcoesters · 2025-12-17T14:16:35Z

tests/test_examples.py

    input_path = _example_path("register_envs")
    for installer, install_dir in create_installer(input_path, tmp_path):
+        if installer.suffix == ".msi":
+            raise Exception("Test for 'register_envs' not yet implemented for MSI")


Should these exceptions be NotImplementedError?

Agree, changed to NotImplementedError

marcoesters · 2026-01-15T14:41:55Z

constructor/briefcase.py

+    if "license_file" in info:
+        return {"file": info["license_file"]}
+    # We cannot return an empty string because that results in an exception on the briefcase side.
+    return {"text": "TODO"}


We do have a placeholder license: https://github.com/conda/constructor/blob/5eecfa0e8b9a46fa96bc19eb1cbf08a91a25fa52/constructor/nsis/placeholder_license.txt

Thanks, updated!

marcoesters · 2026-01-15T14:43:33Z

tests/test_examples.py

+    if is_admin():
+        root_dir = Path(os.environ.get("PROGRAMFILES") or r"C:\Program Files")
+    else:
+        local_dir = os.environ.get("LOCALAPPDATA") or str(Path.home() / r"AppData\Local")


Are you using or instead of default values to account for empty strings?

Correct, updated to follow the convention we use with .get("FOO", <default>)

marcoesters · 2026-01-15T14:45:21Z

tests/test_examples.py

+        process = _execute(cmd, installer_input=installer_input, timeout=timeout, check=check)
+    except subprocess.CalledProcessError as e:
+        if log_path.exists():
+            # When running on the CI system, it gets misdecodes a UTF-16 log file as UTF-8,


Suggested change

# When running on the CI system, it gets misdecodes a UTF-16 log file as UTF-8,

# When running on the CI system, it tries to decode a UTF-16 log file as UTF-8,

marcoesters · 2026-01-15T14:47:08Z

tests/test_examples.py

+            if request:  #  and ON_CI
+                # We always need to do this currently since uninstall doesnt work fully
+                request.addfinalizer(lambda: shutil.rmtree(str(install_dir), ignore_errors=True))


Why not add rmtree to _run_uninstaller_msi then?

Initially I didn't want to the interface to *_uninstaller_msi and *_uninstaller_exe to differ, but since this is temporary I've updated it now, I agree it looks more concise this way.

marcoesters · 2026-01-15T14:48:42Z

tests/test_examples.py

+                if request:
+                    request.addfinalizer(
+                        lambda: shutil.rmtree(str(install_dir), ignore_errors=True)
+                    )


Same here - consider adding to _run_uninstaller_msi, especially since this is used in other tests later.

marcoesters · 2026-01-15T16:46:19Z

tests/test_examples.py

    install_dir: Path,
    timeout: int = 420,
    check: bool = True,
+    request: bool = False,


request is not a boolean, but a FixtureRequest object: https://docs.pytest.org/en/stable/reference/reference.html#request

However, I would say that it's generally not needed as a finalizer. We can just add rmtree as a clean-up step until the uninstallation works well.

Ahh good that you caught that, I should not make assumptions based on an expression like if request:, I've updated it 496bf86, let me know if this is what you were thinking. I think this should be OK now, we always invoke rmtree, hopefully no test depended on rmtree not being called.

lrandersson · 2026-01-15T18:43:16Z

@marcoesters is it possible to restart the CLA/check that failed intermittently?

conda-bot added this to 🔎 Review Dec 9, 2025

github-project-automation bot moved this to 🆕 New in 🔎 Review Dec 9, 2025

lrandersson changed the base branch from main to briefcase-integration December 9, 2025 20:40

conda-bot added the cla-signed [bot] added once the contributor has signed the CLA label Dec 9, 2025

lrandersson force-pushed the briefcase branch from 5d3c3cd to 7073ba6 Compare December 17, 2025 14:08

lrandersson marked this pull request as ready for review December 17, 2025 14:10

lrandersson requested a review from a team as a code owner December 17, 2025 14:10

lrandersson mentioned this pull request Dec 17, 2025

Briefcase uninstall #1143

Closed

3 tasks

marcoesters reviewed Dec 17, 2025

View reviewed changes

marcoesters force-pushed the briefcase-integration branch from 26ee3bb to a2caeec Compare December 19, 2025 17:12

lrandersson force-pushed the briefcase branch from 4be5b94 to 6306743 Compare December 19, 2025 17:18

Add tests

ad8791d

lrandersson force-pushed the briefcase branch from 6306743 to ad8791d Compare December 19, 2025 17:19

lrandersson added 3 commits December 19, 2025 13:24

Some more fixes

61cb821

Test commit to see if this resolves test failure

98ecfaa

Set version back to X also for the other failing test

7130fb9

lrandersson requested a review from marcoesters January 15, 2026 14:02

marcoesters reviewed Jan 15, 2026

View reviewed changes

lrandersson added 3 commits January 15, 2026 10:13

Review fixes

eaf17bb

pre-commit

7df6e9d

Add str conversion

b46c864

marcoesters reviewed Jan 15, 2026

View reviewed changes

Remove request usage for MSI

496bf86

lrandersson requested a review from marcoesters January 15, 2026 21:37

marcoesters approved these changes Jan 15, 2026

View reviewed changes

github-project-automation bot moved this from 🆕 New to ✅ Approved in 🔎 Review Jan 15, 2026

marcoesters merged commit 36a1479 into conda:briefcase-integration Jan 15, 2026
20 of 21 checks passed

github-project-automation bot moved this from ✅ Approved to 🏁 Done in 🔎 Review Jan 15, 2026

	# When running on the CI system, it gets misdecodes a UTF-16 log file as UTF-8,
	# When running on the CI system, it tries to decode a UTF-16 log file as UTF-8,

Conversation

lrandersson commented Dec 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Checklist - did you ...

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!

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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lrandersson Jan 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lrandersson commented Jan 15, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

lrandersson commented Dec 9, 2025 •

edited

Loading

lrandersson Jan 15, 2026 •

edited

Loading