From cec8b7c191afcd34267d2c21584c4f6246a0676f Mon Sep 17 00:00:00 2001 From: "Shane B." Date: Thu, 20 Aug 2026 09:59:36 +0200 Subject: [PATCH] Show Apple's terms of service in the wizard, instead of stopping at them The CLI has been able to do this since the iCloud route landed; the window could not, and stopped at a delegate error with no way past it. Apple takes acceptance on one of its own devices or at icloud.com and nowhere else, so somebody with neither - which is everyone this project is for - was stuck. Same trigger as the CLI: a MobileMeDelegateError, because which error value means "terms pending" is not established, so this looks rather than assumes. Each document is shown whole through the renderer the CLI already uses, and acceptance is sent only for documents that were accepted. complete_login() follows, without which the account is left at AUTHENTICATED - readable, and unusable for everything after it. **One deliberate difference from the CLI, which asks before fetching.** It asks because it is about to page a document at a terminal and cannot take that back. Here the fetch is what decides whether there is anything to show, so asking first would mean offering to look and then reporting there was nothing. A delegate failure with no terms pending reports what Apple said, unchanged, rather than describing the check. Rejecting closes the exporter. That is the one exception to _load's rule that nothing closes the window from here, and it is deliberate: every other failure leaves the button there because trying again might work, and this one cannot - the same document is waiting on the next attempt. Its docstring now says so rather than contradicting the code. **Building the dialog and showing it are two functions, so it can be tested.** grab_set plus wait_window hand control to a nested event loop and a window manager; a test that schedules a click into that deadlocks rather than failing, which is what the first version of this did. Everything worth asserting is in the widgets, and all of it is unreachable once those two lines have run. For the same reason the keyboard bindings are asserted with widget.bind rather than pressed. A Toplevel under a withdrawn root holds focus from nobody, so event_generate is dropped in silence and "press Return, assert nothing happened" passes just as well when the key never arrived. It passed once here and failed on the next run, which is how it was noticed. Fixed-pitch, because terms.render underlines headings with a dash row as long as the heading. Proportional gets every rule wrong, and a contract that looks broken invites the conclusion that it has been tampered with. 28 tests: 13 for the flow with the dialog stubbed, needing no display, and 15 driving the real window. Verified by breaking seven things on purpose - an unanswered dialog counting as agreement, dropping complete_login, sending acceptance before asking, an editable terms box, Return bound to Accept, Reject leaving the window open, and Reject routed through the "please file a bug" handler - each of which reddens the tests that exist for it. **Not verified against Apple.** The trigger needs an account with terms actually pending, which cannot be produced on demand and which nobody here has. The flow tests drive a fake account. CONTRIBUTING said two files need Tk. It was already three before this, and is now four, so that is a table rather than a sentence - plus the two traps above, which are not worth anybody rediscovering. Co-Authored-By: Claude Opus 5 --- CONTRIBUTING.md | 26 +- python/exporter/wizard.py | 258 +++++++++++++++-- python/test/test_terms_dialog.py | 481 +++++++++++++++++++++++++++++++ 3 files changed, 732 insertions(+), 33 deletions(-) create mode 100644 python/test/test_terms_dialog.py diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 15d6eddf..76fa56d7 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -326,10 +326,28 @@ strongest test rebuilds the committed export fixtures in `app/src/test/resources Two things it deliberately does not cover, because they need an Apple account: signing in, and everything the account then yields. What happens to the records afterwards is covered in full. -**Two files need Tk and skip without it** — `test_asyncui.py`, for what happens when somebody -closes the progress window, and `test_wizard_list.py`, for the list of accessories. Both drive -real Tk widgets, because what they cover *is* the event loop and the widget: a cancelled progress -window used to leave the wizard hung for ever, and that is not reproducible against a stand-in. +**Four files need Tk and skip without it.** All of them drive real Tk widgets, because what they +cover *is* the widget: a cancelled progress window used to leave the wizard hung for ever, and +that is not reproducible against a stand-in. + +| File | What it drives | +| --- | --- | +| `test_asyncui.py` | closing the progress window mid-sign-in | +| `test_wizard_list.py` | the list of accessories | +| `test_save_logs_button.py` | the Save logs button, and that no identifier survives into the file | +| `test_terms_dialog.py` | Apple's terms of service, and that refusing them sends nothing | + +**The skip has to be `pytest.importorskip`, at the top, before importing `exporter.wizard`.** A +`pytest.skip` inside a fixture is too late: the module is imported during collection, so a Python +built without Tk fails the whole run before any fixture can decline. + +**Do not test a modal by driving it.** `grab_set` plus `wait_window` hand control to a nested +event loop and a window manager, and a test that schedules a click into that deadlocks rather than +failing. Build the window and show it in separate functions, as `_build_terms_window` and +`_show_terms` do, and drive the built one. For the same reason, keyboard bindings are asserted +with `widget.bind("")` rather than `event_generate`: a Toplevel under a withdrawn root holds +focus from nobody, so a generated keypress is dropped in silence and the test passes without +having pressed anything. **CI runs them on one job in four.** The matrix is 3.10 to 3.13 on `macos-14`, and only its 3.13 has `_tkinter` — the other three skip both files at collection, so a green square there says diff --git a/python/exporter/wizard.py b/python/exporter/wizard.py index 2cb3585a..e4c45a0d 100644 --- a/python/exporter/wizard.py +++ b/python/exporter/wizard.py @@ -32,7 +32,7 @@ from typing import Callable, Sequence from tkinter.filedialog import askopenfilenames, asksaveasfilename -from exporter import icloud, localsource, source +from exporter import icloud, localsource, source, terms from exporter.asyncui import Asker, Cancelled, run_with_progress from exporter.codes import ( VERIFICATION_CODE_LENGTH, @@ -48,7 +48,12 @@ suggested_identifier, suggested_name, ) -from findmy import InvalidCredentialsError +from findmy import ( + InvalidCredentialsError, + LoginState, + MobileMeDelegateError, + TermsError, +) from findmy.keychain.recovery import RecoveryError from exporter.icloud import Candidate, ExportSourceError @@ -79,6 +84,11 @@ TICKED = "\u25a0" UNTICKED = "\u25a1" +# How wide the terms are wrapped, and how wide the box that shows them is. One number rather than +# two, because they have to agree: `terms.render` hard-wraps at whatever it is given, so a box +# narrower than that wraps every line a second time and produces a ragged short line under each. +_TERMS_COLUMNS = 88 + TICKED_ROW = "ticked" """ The tag that colours a ticked row, using this platform's own selection colour. @@ -96,6 +106,19 @@ ] +class TermsDeclined(Exception): + """ + The user read Apple's terms of service and said no. + + **The one answer this window cannot carry on from**, and the only reason it is not just + another `ExportSourceError`. Everything else that goes wrong here leaves the button there to + try again, because trying again is a sensible thing to do. This does not: Apple will not + complete the delegate exchange for an account with terms pending, so a second attempt reaches + the same document and asks the same question. Offering a retry would be pretending the answer + might change by itself. + """ + + class WizardApp(tk.Tk): """ The window: a list of accessories, and a button that writes them. @@ -281,10 +304,13 @@ def _load(self) -> None: """ Read whichever source this machine can use, and add what it holds to the list. - **Nothing closes the window from here.** This runs when a button is pressed rather than - when the program starts, so a sign-in that fails, or one the user changes their mind about - half way through, leaves them where they were - with whatever they had already added from - a key file still in the list, and the button still there to try again. + **Almost nothing closes the window from here.** This runs when a button is pressed rather + than when the program starts, so a sign-in that fails, or one the user changes their mind + about half way through, leaves them where they were - with whatever they had already added + from a key file still in the list, and the button still there to try again. + + The single exception is refusing Apple's terms of service, which is not a failure that + trying again can get past - see :class:`TermsDeclined`. """ try: fetched = ( @@ -295,6 +321,21 @@ def _load(self) -> None: except Cancelled: # They closed the progress window, which says stop this - not close everything. return + except TermsDeclined as declined: + # **The one exception to the paragraph above**, and it is deliberate. Every other + # failure leaves the window open because trying again might work; this one cannot, + # since the same terms are waiting on the next attempt and the answer was no. So it + # says what that means, confirms nothing was sent, and closes. + logger.info("Terms of service declined: %s", declined) + messagebox.showinfo( + "Terms not accepted", + f"Nothing was sent, and your Apple account is unchanged.\n\n" + f"Apple will not let anything read this account until the {declined} terms are" + " accepted, so there is nothing more this exporter can do. You can accept them" + " on an Apple device or at icloud.com, or run this again and read them here.", + ) + self.destroy() + return except (ExportSourceError, ExportError) as e: messagebox.showerror("Could not read your accessories", str(e)) return @@ -367,29 +408,37 @@ async def _read_icloud(self, asker: Asker): if not email or not password: raise ExportSourceError("Signing in was cancelled.") - await icloud.log_in( - account, - email, - password, - choose_second_factor=lambda methods: _async( - asker.ask(lambda: _ask_choice(self, "Verification", "How should Apple send the code?", methods)), - ), - get_code=lambda: _async( - asker.ask(lambda: _ask_string( - self, - "Verification", - f"The {VERIFICATION_CODE_LENGTH}-digit code Apple sent:", - valid=is_verification_code, - transform=verification_code, - )), - ), - retry_credentials=lambda error, attempt: _async( - _ask_again_for_credentials(self, asker, error, attempt), - ), - retry_code=lambda error, attempt: _async( - _ask_again_for_code(self, asker, error, attempt), - ), - ) + try: + await icloud.log_in( + account, + email, + password, + choose_second_factor=lambda methods: _async( + asker.ask(lambda: _ask_choice( + self, "Verification", "How should Apple send the code?", methods, + )), + ), + get_code=lambda: _async( + asker.ask(lambda: _ask_string( + self, + "Verification", + f"The {VERIFICATION_CODE_LENGTH}-digit code Apple sent:", + valid=is_verification_code, + transform=verification_code, + )), + ), + retry_credentials=lambda error, attempt: _async( + _ask_again_for_credentials(self, asker, error, attempt), + ), + retry_code=lambda error, attempt: _async( + _ask_again_for_code(self, asker, error, attempt), + ), + ) + except MobileMeDelegateError as e: + # Authentication worked; the exchange that follows it did not. Unaccepted terms + # are the one cause of that with a remedy here - and which error value means + # "terms pending" is not established, so this looks rather than assumes. + await _accept_pending_terms(self, account, asker, e) # Registered a device by now; remembering it stops the next export registering another. icloud.remember(account) @@ -873,6 +922,157 @@ def _ask_choice(parent: tk.Tk, title: str, prompt: str, options: Sequence[str]) return chosen.get() +def _build_terms_window(parent: tk.Tk, document, index: int, total: int): + """ + Build the terms dialog, and hand back the answer it will write into. + + **Split from :func:`_show_terms` so that it can be tested.** Everything worth checking here is + in the widgets - the document is shown whole, no markup survives, closing means no - and all + of it is unreachable from a test once `grab_set` and `wait_window` have been called, because + those hand control to a nested event loop and a window manager. Building and showing are two + steps for that reason and no other. + + :returns: The window, and a dict whose `value` is the answer. It starts False: the default for + a contract nobody answered has to be the one that sends nothing, so Reject, Escape and the + window's close button all leave it alone and only Accept sets it. + + **Fixed-pitch, and that is not a cosmetic choice.** :func:`exporter.terms.render` wraps to a + column count and underlines its headings with a row of dashes as long as the heading - which + lines up in a terminal and in nothing else. In a proportional font every heading rule comes + out the wrong length, and a contract that looks broken invites the reasonable conclusion that + it has been tampered with. + + Nothing here shortens or summarises: what is on screen is the document Apple sent, because it + is what pressing Accept agrees to. + """ + window = tk.Toplevel(parent) + window.title(f"Apple's terms of service - {document.page_id}") + window.transient(parent) + + frame = ttk.Frame(window, padding=16) + frame.pack(fill="both", expand=True) + frame.grid_columnconfigure(0, weight=1) + frame.grid_rowconfigure(1, weight=1) + + # Said above the document rather than after it: somebody who has just typed a password and is + # suddenly looking at a contract needs to know why before they start reading it. + counted = f" ({index} of {total})" if total > 1 else "" + ttk.Label( + frame, + text=( + f"Apple will not finish signing you in until these terms are accepted{counted}." + " They are shown in full - this is what you would be agreeing to.\n\n" + "Accepting records your agreement on your Apple account. Nothing has been sent yet." + ), + wraplength=560, + justify="left", + ).grid(row=0, column=0, columnspan=2, sticky="w", pady=(0, 12)) + + text = tk.Text(frame, wrap="word", width=_TERMS_COLUMNS, height=28, font="TkFixedFont") + text.grid(row=1, column=0, sticky="nsew") + + scrollbar = ttk.Scrollbar(frame, orient="vertical", command=text.yview) + scrollbar.grid(row=1, column=1, sticky="ns") + text.configure(yscrollcommand=scrollbar.set) + + text.insert("1.0", terms.render(document.html, _TERMS_COLUMNS)) + # Read-only rather than merely discouraged. A Text is editable by default, and a contract you + # can type into is not the document that was fetched. + text.configure(state="disabled") + + accepted = {"value": False} + + def _accept() -> None: + accepted["value"] = True + window.destroy() + + buttons = ttk.Frame(frame) + buttons.grid(row=2, column=0, columnspan=2, sticky="e", pady=(12, 0)) + ttk.Button(buttons, text="Reject", command=window.destroy).pack(side="right", padx=(8, 0)) + ttk.Button(buttons, text="Accept", command=_accept).pack(side="right") + + # No Return binding, deliberately. Every other dialog here submits on Return because its + # answer is something the user typed; this one's answer is agreement to a contract, and a + # stray keypress landing on it is not agreement. + window.bind("", lambda _event: window.destroy()) + + return window, accepted + + +def _show_terms(parent: tk.Tk, document, index: int, total: int) -> bool: + """ + Show one terms document in full, and wait for it to be agreed to or refused. + + :returns: True only if Accept was pressed. See :func:`_build_terms_window`, which is where + everything except the waiting lives. + """ + window, accepted = _build_terms_window(parent, document, index, total) + + centre_over(window, parent) + window.grab_set() + parent.wait_window(window) + + return accepted["value"] + + +async def _accept_pending_terms(parent: tk.Tk, account, asker: Asker, error) -> None: + """ + Show whatever Apple wants agreeing to, and finish the sign-in if it is agreed to. + + **Only reached when signing in has already failed** on the delegate exchange, which is what an + account with unaccepted terms does. Apple takes acceptance on one of its own devices or on + iCloud.com and nowhere else, so somebody with neither is stuck without this - which is the + whole reason the CLI grew it, and there was no reason for the window not to have it too. + + **Unlike the CLI, this does not ask permission to fetch first.** The CLI asks because it is + about to page a document at a terminal and cannot take that back. Here the fetch decides + whether there is anything to show at all, so asking first would mean offering to look and then + reporting that there was nothing - where simply looking reports Apple's own message unchanged. + + :param error: What the delegate exchange said. Carried so that a failure for some other reason + is reported as Apple worded it, rather than as "no terms found". + :raises TermsDeclined: If any document is rejected. Nothing is sent for it, and no later + document is shown. + """ + try: + documents = await account.fetch_terms() + except TermsError as e: + raise ExportSourceError( + f"Signing in stopped at your account:\n\n{error}\n\n" + f"Asking Apple which terms are pending also failed:\n\n{e}", + ) from e + + if not documents: + # Signing in failed for some other reason, and accepting nothing would not fix it. Apple's + # own words, because they are the only description of the actual problem anyone has. + raise ExportSourceError( + f"Signing in got as far as your account and then stopped:\n\n{error}\n\n" + "There are no terms of service waiting to be accepted, so this is something else.", + ) + + for index, document in enumerate(documents, start=1): + # Bound as a default argument: the lambda runs on the main thread after this iteration has + # moved on, and a closure over the loop variable would show the last document every time. + agreed = asker.ask( + lambda d=document, i=index: _show_terms(parent, d, i, len(documents)), + ) + if not agreed: + raise TermsDeclined(document.page_id) + + try: + await account.accept_terms(document) + except TermsError as e: + raise ExportSourceError( + f"Apple did not record agreement to the {document.page_id} terms:\n\n{e}", + ) from e + + # The step `login` would have run itself had the terms not been pending. Without it the + # account is left at AUTHENTICATED - readable, and unusable for everything after this. + state = await account.complete_login() + if state != LoginState.LOGGED_IN: + raise ExportSourceError(f"The terms were accepted, but signing in ended at {state}.") + + async def _async(value): """Wrap an already-computed answer, for the awaitable callbacks `icloud.log_in` expects.""" return value diff --git a/python/test/test_terms_dialog.py b/python/test/test_terms_dialog.py new file mode 100644 index 00000000..0d8e0f02 --- /dev/null +++ b/python/test/test_terms_dialog.py @@ -0,0 +1,481 @@ +""" +Apple's terms of service, in the window. + +**Two halves, tested apart.** `_accept_pending_terms` is the flow - what gets fetched, what gets +sent, and what happens when the answer is no; it runs with the dialog stubbed, so it needs no +display. `_show_terms` is the window, and is driven for real, because the failure that matters +there is one nothing throws on: a dialog that shows the wrong document, or that treats a closed +window as agreement. + +The second kind is why this exists at all. Sending acceptance of a contract the user rejected +would be silent, permanent and done in their name. +""" + +from __future__ import annotations + +import asyncio +from dataclasses import dataclass +from typing import Any +from unittest import mock + +import pytest + +# Before anything that imports tkinter, `exporter.wizard` included - see test_save_logs_button. +tk = pytest.importorskip("tkinter", reason="needs a Python built with Tk") + +from tkinter import ttk # noqa: E402 - same reason + +from exporter import wizard # noqa: E402 - has to follow the importorskip above +from exporter.icloud import ExportSourceError # noqa: E402 +from findmy import LoginState, MobileMeDelegateError, TermsError # noqa: E402 + +TERMS_HTML = """ + + +
+

iCloud Terms and Conditions

+

Welcome to iCloud. By using iCloud you agree to these terms.

+

1. Your Account

+

You are responsible for maintaining the confidentiality of your account.

+
    +
  • You must be over the age of majority.
    • +
+ +
+ +""" + +MEDIA_HTML = "

Media Services

A second document.

" + +DELEGATE_ERROR = MobileMeDelegateError(localized_error="Terms have not been accepted") + + +@dataclass(frozen=True) +class FakeTerms: + page_id: str + agree_url: str + html: str + + +ICLOUD_TERMS = FakeTerms("iCloud", "https://example.invalid/agree", TERMS_HTML) +MEDIA_TERMS = FakeTerms("Media", "https://example.invalid/agree-media", MEDIA_HTML) + + +class FakeAccount: + """ + An account with terms pending, recording only what actually reached Apple. + + `accepted` is the assertion that matters throughout: it is the list of documents this + program told Apple the user agreed to. + """ + + def __init__(self, documents=(ICLOUD_TERMS,), *, final_state=LoginState.LOGGED_IN): + self.documents = list(documents) + self.accepted: list[FakeTerms] = [] + self.completed = 0 + self.final_state = final_state + self.fetch_raises: Exception | None = None + self.accept_raises: Exception | None = None + + async def fetch_terms(self): + if self.fetch_raises: + raise self.fetch_raises + return self.documents + + async def accept_terms(self, document): + if self.accept_raises: + raise self.accept_raises + self.accepted.append(document) + + async def complete_login(self): + self.completed += 1 + return self.final_state + + +class DirectAsker(wizard.Asker): + """ + An `Asker` that answers on the calling thread, since these tests have no worker. + + A real one hands the question to the main thread and blocks until an event loop draws it, + which needs the worker thread that is not here. `ask` is the only method the flow uses, so + nothing the base class sets up is wanted - hence no `super().__init__`. + """ + + def __init__(self): + pass + + def ask(self, dialog): + return dialog() + + +def run_terms(account, answers, monkeypatch, parent: Any = None): + """Drive the flow with canned answers to each document, in order.""" + given = list(answers) + shown: list[FakeTerms] = [] + + def _fake_dialog(_parent, document, _index, _total): + shown.append(document) + return given.pop(0) + + monkeypatch.setattr(wizard, "_show_terms", _fake_dialog) + + asyncio.run(wizard._accept_pending_terms(parent, account, DirectAsker(), DELEGATE_ERROR)) + + return shown + + +class TestAgreeing: + def test_accepting_sends_agreement_for_that_document(self, monkeypatch): + account = FakeAccount() + + run_terms(account, [True], monkeypatch=monkeypatch) + + assert account.accepted == [ICLOUD_TERMS] + + def test_the_sign_in_is_finished_afterwards(self, monkeypatch): + # Without this the account is left at AUTHENTICATED - readable, and unusable for + # everything that comes after. It is the step `login` would have run itself. + account = FakeAccount() + + run_terms(account, [True], monkeypatch=monkeypatch) + + assert account.completed == 1 + + def test_every_document_is_shown_and_accepted(self, monkeypatch): + account = FakeAccount([ICLOUD_TERMS, MEDIA_TERMS]) + + shown = run_terms(account, [True, True], monkeypatch=monkeypatch) + + assert shown == [ICLOUD_TERMS, MEDIA_TERMS] + assert account.accepted == [ICLOUD_TERMS, MEDIA_TERMS] + + def test_a_sign_in_that_does_not_complete_is_reported(self, monkeypatch): + account = FakeAccount(final_state=LoginState.AUTHENTICATED) + + with pytest.raises(ExportSourceError, match="ended at"): + run_terms(account, [True], monkeypatch=monkeypatch) + + +class TestRefusing: + def test_rejecting_sends_nothing(self, monkeypatch): + # The whole point. Acceptance is permanent and made in the user's name. + account = FakeAccount() + + with pytest.raises(wizard.TermsDeclined): + run_terms(account, [False], monkeypatch=monkeypatch) + + assert account.accepted == [] + + def test_rejecting_does_not_finish_the_sign_in(self, monkeypatch): + account = FakeAccount() + + with pytest.raises(wizard.TermsDeclined): + run_terms(account, [False], monkeypatch=monkeypatch) + + assert account.completed == 0 + + def test_rejecting_the_first_document_does_not_show_the_second(self, monkeypatch): + account = FakeAccount([ICLOUD_TERMS, MEDIA_TERMS]) + + with pytest.raises(wizard.TermsDeclined): + run_terms(account, [False, True], monkeypatch=monkeypatch) + + assert account.accepted == [] + + def test_accepting_the_first_and_refusing_the_second_leaves_the_first_accepted(self, monkeypatch): + # Honest about what happened rather than tidy: the first agreement was sent the moment it + # was given, and cannot be taken back by refusing a later one. + account = FakeAccount([ICLOUD_TERMS, MEDIA_TERMS]) + + with pytest.raises(wizard.TermsDeclined): + run_terms(account, [True, False], monkeypatch=monkeypatch) + + assert account.accepted == [ICLOUD_TERMS] + assert account.completed == 0 + + def test_it_names_the_document_that_was_refused(self, monkeypatch): + account = FakeAccount([ICLOUD_TERMS]) + + with pytest.raises(wizard.TermsDeclined, match="iCloud"): + run_terms(account, [False], monkeypatch=monkeypatch) + + +class TestWhenItIsNotTermsAtAll: + """ + Which error means "terms pending" is not established, so the flow looks rather than assumes. + + A delegate failure with nothing to accept has to report what Apple said. Reporting "no terms + found" would describe the check rather than the problem. + """ + + def test_no_pending_terms_reports_what_apple_said(self, monkeypatch): + account = FakeAccount([]) + + with pytest.raises(ExportSourceError, match="Terms have not been accepted"): + run_terms(account, [], monkeypatch=monkeypatch) + + def test_no_pending_terms_is_not_a_declined_terms_close(self, monkeypatch): + # It must not close the window: nothing was refused, and trying again may well work. + account = FakeAccount([]) + + with pytest.raises(ExportSourceError): + run_terms(account, [], monkeypatch=monkeypatch) + + def test_a_failed_fetch_carries_both_messages(self, monkeypatch): + account = FakeAccount() + account.fetch_raises = TermsError("no terms in the response") + + with pytest.raises(ExportSourceError) as raised: + run_terms(account, [], monkeypatch=monkeypatch) + + assert "Terms have not been accepted" in str(raised.value) + assert "no terms in the response" in str(raised.value) + + def test_a_rejected_acceptance_is_reported_rather_than_ignored(self, monkeypatch): + # Apple refusing to record the agreement is not the same as it being recorded, and the + # sign-in must not carry on as though it were. + account = FakeAccount() + account.accept_raises = TermsError("status 500") + + with pytest.raises(ExportSourceError, match="did not record agreement"): + run_terms(account, [True], monkeypatch=monkeypatch) + + assert account.completed == 0 + + +# ------------------------------------------------------------------------------------------------ +# The window itself +# ------------------------------------------------------------------------------------------------ + + +@pytest.fixture(scope="module") +def root(): + """ + One root for the whole module - see the same fixture in test_save_logs_button for why. + + A bare `Tk` rather than a `WizardApp`: the dialog only uses its parent to own the window, so + building the whole wizard around it would be testing the wizard. + """ + try: + app = tk.Tk() + except tk.TclError as e: # pragma: no cover - depends on the machine, not on the code + pytest.skip(f"needs a display to build a window: {e}") + + app.withdraw() + try: + yield app + finally: + app.destroy() + + +@pytest.fixture +def dialog(root): + """The real terms window, built but not yet modal, with the answer it writes into.""" + window, accepted = wizard._build_terms_window(root, ICLOUD_TERMS, 1, 1) + try: + yield window, accepted + finally: + if window.winfo_exists(): + window.destroy() + + +def descendants(widget): + yield widget + for child in widget.winfo_children(): + yield from descendants(child) + + +def terms_box(window): + for widget in descendants(window): + if isinstance(widget, tk.Text): + return widget + raise AssertionError("the dialog has nowhere to show the terms") + + +def shown_text(window) -> str: + return terms_box(window).get("1.0", "end") + + +def blurb(window) -> str: + return " ".join( + str(widget.cget("text")) + for widget in descendants(window) + if isinstance(widget, ttk.Label) + ) + + +def still_open(app) -> bool: + """ + Whether the application is still there. + + Destroying a *root* tears down its Tcl interpreter, so `winfo_exists` does not come back + False - it raises, because there is no longer an interpreter to ask. Anything checking that + the exporter closed has to treat that as the answer rather than as an error. + """ + try: + return bool(app.winfo_exists()) + except tk.TclError: + return False + + +def press(window, label) -> None: + for widget in descendants(window): + if isinstance(widget, ttk.Button) and str(widget.cget("text")) == label: + widget.invoke() + return + + raise AssertionError(f"the dialog has no {label!r} button") + + +class TestTheDialogsAnswer: + def test_accept_agrees(self, dialog): + window, accepted = dialog + + press(window, "Accept") + + assert accepted["value"] is True + + def test_reject_does_not(self, dialog): + window, accepted = dialog + + press(window, "Reject") + + assert accepted["value"] is False + + def test_both_buttons_close_the_window(self, dialog): + # A dialog still on screen after its answer was given reads as the click not registering, + # and the next click lands on whatever replaced it. + window, _ = dialog + + press(window, "Reject") + + assert not window.winfo_exists() + + def test_an_unanswered_dialog_has_not_agreed(self, dialog): + # What the window's own close button leaves behind. `_show_terms` returns this value, so + # closing a contract unanswered has to read as no. + _, accepted = dialog + + assert accepted["value"] is False + + def test_escape_can_dismiss_it(self, dialog): + window, _ = dialog + + assert window.bind(""), "no way out of this dialog from the keyboard" + + def test_nothing_is_bound_to_return(self, dialog): + """ + Every other dialog here submits on Return. This one must not: its answer is agreement to a + contract, and a stray keypress is not agreement. + + **Asserted on the binding rather than by pressing the key**, which is not a shortcut. A + Toplevel whose root is withdrawn is not viewable and so holds focus from nobody, and + `event_generate` on an unfocused window is dropped without complaint - so a test that + presses Return and checks nothing happened passes just as well when the key was never + delivered. It did, here, until the same call started failing a run later. + """ + window, _ = dialog + + assert not window.bind("") + assert not window.bind("") + + +class TestWhatIsOnScreen: + def test_the_document_is_shown(self, dialog): + window, _ = dialog + + assert "By using iCloud you agree to these terms." in shown_text(window) + assert "You are responsible for maintaining the confidentiality" in shown_text(window) + + def test_list_items_survive(self, dialog): + window, _ = dialog + + assert "You must be over the age of majority." in shown_text(window) + + def test_no_markup_reaches_the_reader(self, dialog): + window, _ = dialog + text = shown_text(window) + + assert "

" not in text + assert "" not in text + # A contract with someone's analytics in the middle of it reads as a forgery. + assert "track();" not in text + assert "color: red" not in text + + def test_the_terms_cannot_be_edited(self, dialog): + # A Text is editable by default, and a contract you can type into is not the document + # that was fetched. + window, _ = dialog + + assert str(terms_box(window).cget("state")) == "disabled" + + def test_it_says_apple_is_waiting_on_this(self, dialog): + window, _ = dialog + + assert "will not finish signing you in" in blurb(window) + assert "Nothing has been sent yet" in blurb(window) + + def test_one_document_is_not_counted(self, dialog): + window, _ = dialog + + assert "(1 of 1)" not in blurb(window) + + def test_several_documents_are_counted(self, root): + # Otherwise accepting the first looks like the whole thing, and a second contract appears + # from nowhere. + window, _ = wizard._build_terms_window(root, MEDIA_TERMS, 2, 2) + + try: + assert "(2 of 2)" in blurb(window) + assert "A second document." in shown_text(window) + assert "By using iCloud" not in shown_text(window) + finally: + window.destroy() + + def test_the_title_names_the_document(self, root): + window, _ = wizard._build_terms_window(root, MEDIA_TERMS, 2, 2) + + try: + assert "Media" in window.title() + finally: + window.destroy() + + +class TestRejectingClosesTheExporter: + """ + The one failure this window does not offer to retry. + + Everything else that goes wrong during a sign-in leaves the button there, because trying + again might work. Refusing a contract is not like that: the same document is waiting on the + next attempt, so offering a retry would be pretending the answer might change by itself. + """ + + def test_it_says_nothing_was_sent_and_then_closes(self, monkeypatch): + app = wizard.WizardApp() + app.withdraw() + + def _declined(*_args, **_kwargs): + raise wizard.TermsDeclined("iCloud") + + monkeypatch.setattr(wizard, "run_with_progress", _declined) + + try: + with mock.patch.object(wizard.messagebox, "showinfo") as info, \ + mock.patch.object(wizard.messagebox, "showerror") as error: + app._load() + + # Not the generic handler, which asks for a bug report with a log attached. Somebody + # who read a contract and declined it has not hit a bug. + assert not error.called, "declining terms is not something to report" + + body = info.call_args[0][1] + assert "Nothing was sent" in body + assert "unchanged" in body + assert "iCloud" in body, "say which document, since there may have been several" + # Where they can accept, for somebody who declined by accident or changed their mind. + assert "icloud.com" in body + + assert not still_open(app), "Reject has to close the exporter" + finally: + if still_open(app): + app.destroy()