diff --git a/.claude/CLAUDE.md b/.claude/CLAUDE.md index 168b840..e50640b 100644 --- a/.claude/CLAUDE.md +++ b/.claude/CLAUDE.md @@ -35,14 +35,14 @@ Review code against these criteria, whether on its own or in the TDD refactor st - **Comments**: A docstring is one line. A docstring is only more than one line if something needs explaining the code cannot. A module's docstring says what the module holds rather than repeating its main class. A docstring or comment says what this code does now. It never says where the increment is heading, and never repeats the signature. Delete a sentence that narrates or justifies an edit, that walks through the steps in order, that says what the code is not, or that restates a parameter, a return type, or a default. Delete a sentence that says what the return value is for: the summary already names it. Delete a sentence that says what a caller does, or that names a concept from a layer further out. A test's docstring says which case the test pins; the reason for the behaviour belongs in the code under test and in the README. Keep a contrast only when the reader has to act on the difference. When a signature changes, the summary usually needs one word, not a new sentence. A docstring that matches the one beside it was copied rather than checked, so read both. - **Duplication**: Look for the same decision taken in more than one place, not for repeated lines. Count what the fix adds against what it removes before you write it: a shared version needing a parameter for everything that differs is repeated lines. A rule every module has to remember is duplication too, so state it once, where nobody can forget it. A helper that callers have to remember to call can be forgotten as well, so prefer a check that reads the code itself. In tests, turn repeated setup or a repeated assertion into a named helper. -- **Reuse**: Look for an existing type, test helper, or fixture before you write a new one. Watch for three misses: a fixture's value spelled out as a literal, a mixin's setup redone inline, and the same builder written in two modules instead of in the shared one. +- **Reuse**: Look for an existing type, test helper, or fixture before you write a new one. Watch for four misses: a fixture's value spelled out as a literal, a mixin's setup redone inline, the same builder written in two modules instead of in the shared one, and an inline expression or check that a function in the same module already computes. Grep the module for it. - **Complexity**: A function holds one decision. Watch for nesting, for a flag parameter that makes one function do two things, and for a long parameter list. When a docstring needs several sentences for the control flow, the code does too much. - **Missing abstractions**: Values that always travel together want a type. A sequence of calls that callers have to make in the right order wants a name. Raw strings, tuples, and dicts standing in for a domain concept are the usual smell, whether that type exists already or still has to be written. - **Visibility**: A class, function, method, or constant without a leading underscore claims callers outside its own module or class. Check that it has them. A name made public for a call site that has since changed is how that claim goes stale. - **Failure paths**: For every call that can fail — a request, a parse, a subprocess — check what happens when it does, and mock it so that it fails the same way. A mock that answers where the real thing raises lets a test pass without the branch it is named for. Log every exception you catch, or the failure is invisible. An exception that escapes aborts the whole run over one bad reference. A write that fails partway has to leave the file as it was. - **Cost**: The work here is network requests, so count them. Watch for a request added inside a per-reference loop, and for a call path that bypasses a cache the old one used. - **Readability**: Use the domain vocabulary that the code and the README establish. Use it in code, docstrings, log messages, and tests alike, and never coin a second word for a concept one of them already names. Prefer an early return to a nested conditional. Say what a test asserts in its method name. Keep implementation terms out of anything the user reads. When a behaviour is described in more than one place, change every place, not only the one you are in. -- **Prose**: Write plainly — in docstrings, comments, log messages, CLI help, the README, and your replies to me. One sentence, one claim. Put the subject first and the verb right after it. Split a sentence instead of nesting a clause in it. Name the thing when `it` or `which` could point at two things. Don't stack negatives. Don't rank; saying for example "the least," "the only," "the one thing." Give the concrete case, not the general rule. Don't skip a step in an explanation. Read each new sentence again, and rewrite it if it needs a second read. +- **Prose**: Write plainly — in docstrings, comments, log messages, CLI help, the README, and your replies to me. One sentence, one claim. Put the subject first and the verb right after it. Split a sentence instead of nesting a clause in it. Name the thing when `it` or `which` could point at two things. Don't stack negatives. Don't rank; saying for example "the least," "the only," "the one thing." Give the concrete case, not the general rule. Don't skip a step in an explanation. Read each new sentence again, in context, and rewrite it if it needs a second read. ## TDD @@ -87,7 +87,7 @@ A few rules that keep the cycle honest: 2. Build a behaviour before its off-switch. Don't test an opt-out, a flag, or any other suppression until the thing it suppresses exists. 3. Assert what happens and what doesn't. An assertion runs on every run; a claim in a docstring is checked by nobody. - A test that asserts nothing was found also passes when nothing was examined, so assert that something was. - - Neither a test's name nor a green run is evidence of what the test guards. Settle that with `just mutate`, for a duplicate you would fold or delete as much as for anything else. When it says a case guards nothing its neighbours don't, delete it, and reshape the test that leaves behind. Register the mutation with `@kills` only where that test is what kills it, and leave it unregistered where the suite kills it anyway. + - Neither a test's name nor a green run is evidence of what the test guards. Settle that with `just mutate`: for a duplicate you would fold or delete, and for a candidate you propose to drop from the list, as much as for anything else. When it says a case guards nothing its neighbours don't, delete it, and reshape the test that leaves behind. Register the mutation with `@kills` only where that test is what kills it, and leave it unregistered where the suite kills it anyway. - Call a stub that varies its answer by an argument with more than one value of that argument, or the test shows something other than what its name claims. The same holds for a fixture whose docstring names a case it does not create. - Pick the mutation from the regression the guard defends against, not from the nearest line to mutate. One that leaves the guard green says nothing about it. One that fails a dozen other tests says little more: it shows the suite reacting, not that guard. - When a stub quotes more than a handful of lines, look for a shorter form that isolates the same regression. @@ -110,27 +110,28 @@ A few rules that keep the cycle honest: - When only CI can decide, try each option locally and say what stays unverified. - Don't announce a comparison and then not run it. - A probe is evidence only when it would fail if the answer were the other way. A rule that matched nothing, or a probe whose signal the code under test swallows, passes exactly like one that holds. + - A sample settles a decision only when it holds the case the decision turns on. Look for that case before you recommend. 9. Check a claim against the code or the tools before you state it. One run or a look at a sibling module usually settles one, such as "nothing covers this yet." - - A review finding is a claim, and the most plausible findings are the ones to check hardest. Mutate the code the finding describes and say what that showed, or don't report it. + - A review finding is a claim, and the most plausible findings are the ones to check hardest. Mutate the code the finding describes and say what that showed, or don't report it. A reviewer's count or estimate is a claim too: measure it before you put it to me. - Anything an issue says is a claim too, whether you carry out its instruction or copy its sentence into the README. A spec says what was intended, not what got built. - So is the reason you give for an option you put to me, because I choose on that reason. - Measure a challenged claim. Don't argue it. 10. Add a missing candidate test to the list as soon as you find one, at whatever step you are in. 11. A refactor that changes behaviour is not a refactor, however unreachable the changed case looks. A change that only *adds* behaviour counts as well: a decorator that fills in methods the class was missing changes what calling them does. Say so before you make the change, not after, and let me decide whether a test has to drive it first. 12. A problem you hit and fixed yourself needs no narration, at whatever step it happens: a probe that misfired, a rewrite that overreached, a formatter that undid an edit. That holds for the cycle's report and for the session's edits as much as for the work itself. Note it, and bring it to the session's evaluation if it still matters. -13. An answer of mine may admit more than one reading. Name the readings you see and ask, rather than implementing the one you would pick: a message costs less than the cycle that undoes a guess. When I push back on one passage twice, we are working from different assumptions. Name yours and ask for mine, rather than rewriting the passage again. +13. An answer of mine may admit more than one reading. Name the readings you see and ask, rather than implementing the one you would pick: a message costs less than the cycle that undoes a guess. When I push back on one passage twice, we are working from different assumptions. Name yours and ask for mine, rather than rewriting the passage again. A question I leave unanswered next to your recommendation is answered by the recommendation. When every candidate test passes, propose an increment review before you propose the self-improvement session. The increment is the one an issue lists, or the whole session where no issue lists one. Review everything the increment changed, against the same criteria, and report it as its own numbered list. Read the files it touched end to end rather than its diff: a finding can span cycles and show up in no single one, such as a helper the second cycle duplicated, a module head grown long with constants, or a name that restates the line beside it. -A bug fix or a small diff gets that one review. An increment of several cycles gets a second review with fresh context, from subagents you give the criteria and the diff but not the reasoning that produced the code, because a review in the context that wrote the code misses what a fresh one catches. Run each in a git worktree of its own, so the mutations they run rewrite neither your tree nor each other's. Tell the subagents to check each finding against the code before reporting it, and judge what they report against the criteria yourself before it reaches my list. +A bug fix or a small diff gets that one review. An increment of several cycles gets a second review with fresh context, from subagents you give the criteria and the diff but not the reasoning that produced the code, because a review in the context that wrote the code misses what a fresh one catches. Run each in a git worktree of its own, so the mutations they run rewrite neither your tree nor each other's. A worktree starts at the default branch, so tell each subagent to `git checkout --detach` the branch head first. Remove the worktrees and their branches once they report. Tell the subagents to check each finding against the code before reporting it, and judge what they report against the criteria yourself before it reaches my list. Merge both reviews into one numbered list. A finding only one review reached belongs in it as much as one they both reached, which you state once. Every finding sits in that list, so each can be referred to by its number, and the text around the list holds none. Group the list, the findings that change behaviour first and the rest by file or subject. ## Documentation -- README.md is generated. Edit `docs/README.md.in` and regenerate with `just readme`. An edit to README.md itself is lost on the next run, without a word. Its per-type headings are questions, so keep each section's sentences answering its own question. +- README.md is generated. Edit `docs/README.md.in` and regenerate with `just readme`. An edit to README.md itself is lost on the next run, without a word. Its per-type headings are questions, so keep each section's sentences answering its own question. After a cycle adds to a section, reread the whole section as someone asking its question, and rewrite it when the additions no longer answer that question in order. - A change to what Update-time does gets a changelog entry under `[Unreleased]`: one line naming the behaviour and linking the issue. A change to the documentation alone gets none. - The detail belongs in the README, not in the changelog. The README names the behaviour and shows the message it produces. It leaves out a worked example of what that message already shows. - When a change lands over several cycles, update the README and the changelog once the behaviour has settled, and before you hand the increment back. Neither may document a state the code is not in. A command-line option is one such state: the cycle that adds it does everything its help promises, rather than accepting a value it then ignores. diff --git a/CHANGELOG.md b/CHANGELOG.md index 66009b6..0a9daa4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,7 +6,13 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/) ## [Unreleased] -No changes yet. +### Added + +- Show what changed in the new version of each Maven dependency that Maven updated. Closes [#374](https://github.com/ICTU/update-time/issues/374). + +### Changed + +- Check a Maven dependency for archival when its pom names its GitHub repository in the project's own `` rather than in ``, or leaves it to the parent pom. Closes [#374](https://github.com/ICTU/update-time/issues/374). ## 0.0.39 - 2026-09-23 diff --git a/README.md b/README.md index 61f8e40..a51e81a 100644 --- a/README.md +++ b/README.md @@ -1073,7 +1073,7 @@ Update-time holds nothing back for two kinds of dependency. Maven Central does n Update-time checks each dependency against the newest release Maven Central lists for it. The pom's plugins are checked too. -Update-time checks a dependency or plugin that the pom declares a `` for. It asks Maven Central about an artefact by that artefact's `groupId:artifactId` coordinates alone, so it still checks a dependency whose `` names a property its parent declares. Update-time reads each pom on its own, however, so it does not check a dependency whose group or artifact names a property its parent declares. +Update-time checks a dependency or plugin that the pom declares a `` for. It asks Maven Central about an artefact by that artefact's `groupId:artifactId` coordinates alone, so it still checks a dependency whose `` names a property its parent declares. It does not check a dependency whose group or artifact names a property its parent declares, since Update-time does not resolve the properties a parent pom declares. Maven Central lists nothing for an artefact a project resolves from a mirror or a private repository. Update-time then has nothing to measure staleness against. @@ -1085,19 +1085,25 @@ Maven Central does not report a withdrawal, so Update-time never warns that a Ma Update-time checks the pom's dependencies against OSV's Maven advisories. The version checked is the one the pom holds once Maven has run, so a vulnerability the run updated away from is never reported. -Update-time does not check a dependency when its group, artifact, or version names a property its parent declares. Update-time reads each pom on its own, so it leaves a property a parent declares as the pom wrote it. OSV matches an advisory to `groupId:artifactId` coordinates and a version, and a property name is neither of those. +Update-time does not check a dependency for vulnerabilities at all when its group, artifact, or version names a property its parent declares. Update-time does not resolve the properties a parent pom declares. OSV matches an advisory to `groupId:artifactId` coordinates and a version, and a property name is neither of those. Update-time does not check a dependency whose `` element is empty either, since OSV needs a version to match an advisory to. -Silencing this warning for a single dependency would need a marker, and Update-time does not yet support markers for Maven dependencies. Pass `--ignore-vulnerability` to silence an advisory across the run instead, or `--vulnerability-level` to only hear about the more severe ones. +Silencing the vulnerability warning for a single dependency would need a marker, and Update-time does not yet support markers for Maven dependencies. Pass `--ignore-vulnerability` to silence an advisory across the run instead, or `--vulnerability-level` to only hear about the more severe ones. #### Archived dependencies -Maven Central does not publish an archival signal of its own. It does serve the pom each version was published with. A pom's `` element may name the repository the project's source lives in. So Update-time reads that element. Where it names a GitHub repository, Update-time warns when GitHub reports that repository as archived. The pom's plugins are checked too. +Maven Central does not publish an archival signal of its own. So Update-time looks up the project's repository on GitHub, and warns when GitHub reports that repository as archived. It checks the pom's plugins too. -The pom read is the one beside the artefact's newest release, since archival is a fact about the project rather than about the version a reference pins. The repository is taken from the `` element's ``, ``, or ``, whichever of them names a GitHub repository first. +Update-time finds the repository in the pom that Maven Central serves beside the artefact's newest release. It reads the newest release's pom whatever version the reference pins, because archival is a fact about the project rather than about one of its versions. Update-time looks in these places, in this order, and takes the first one that names a GitHub repository: -Update-time warns only where it finds a GitHub repository to ask about. The pom it reads may not declare an `` element, as `com.google.guava:guava`'s does not. A project whose parent pom holds the `` reads the same way, since Update-time reads the artefact's own pom alone. An `` may name a host other than GitHub, as `org.apache.commons:commons-lang3` names `gitbox.apache.org`. And Maven Central may not serve a pom at all, for an artefact a project resolves from a mirror or a private repository. Update-time warns about a pom it cannot fetch or cannot parse, and does not take a repository from either. +1. The ``, ``, and `` of the `` element of the newest release's pom. That element names where the project's source lives. +2. The `` element of the newest release's pom, although it names the project's website more often than its repository. Update-time tries it after ``, because a module's `` may name the repository of the larger project the module is part of. The `` of `com.fasterxml.jackson.core:jackson-databind` names `FasterXML/jackson`, while its `` names `FasterXML/jackson-databind`. +3. The `` and `` of the parent pom that the `` element of the newest release's pom names. A project that consists of several modules often names its repository once, in the pom that all its modules inherit from. Update-time reads the parent pom only when the `` element of the newest release's pom does not name a URL. -Silencing this warning for a single dependency would need a marker, and Update-time does not yet support markers for Maven dependencies. Pass `--ignore-archived` to switch the check off for the whole run instead. +Update-time reads one parent pom, and does not read the parent pom's own parent. The poms higher up usually belong to an organisation rather than to a project. Every Apache project inherits from `org.apache:apache`, for example, which names the repository `apache/maven-apache-parent`. For the same reason, Update-time does not read the parent pom when the `` element of the newest release's pom names a repository outside GitHub. That parent is often an organisation's pom, too. + +Update-time warns only where it finds a GitHub repository to ask about. The newest release's pom and its parent pom may leave the repository unnamed. They may also name a host other than GitHub, as `org.apache.commons:commons-lang3` names `gitbox.apache.org`. And Maven Central may not serve a pom at all, for an artefact a project resolves from a mirror or a private repository. Update-time warns about a pom it cannot fetch or cannot parse. + +Silencing the archival warning for a single dependency would need a marker, and Update-time does not yet support markers for Maven dependencies. Pass `--ignore-archived` to switch the check off for the whole run instead. #### Markers diff --git a/docs/README.md.in b/docs/README.md.in index b4cea88..d2653d1 100644 --- a/docs/README.md.in +++ b/docs/README.md.in @@ -931,7 +931,7 @@ Update-time holds nothing back for two kinds of dependency. Maven Central does n Update-time checks each dependency against the newest release Maven Central lists for it. The pom's plugins are checked too. -Update-time checks a dependency or plugin that the pom declares a `` for. It asks Maven Central about an artefact by that artefact's `groupId:artifactId` coordinates alone, so it still checks a dependency whose `` names a property its parent declares. Update-time reads each pom on its own, however, so it does not check a dependency whose group or artifact names a property its parent declares. +Update-time checks a dependency or plugin that the pom declares a `` for. It asks Maven Central about an artefact by that artefact's `groupId:artifactId` coordinates alone, so it still checks a dependency whose `` names a property its parent declares. It does not check a dependency whose group or artifact names a property its parent declares, since Update-time does not resolve the properties a parent pom declares. Maven Central lists nothing for an artefact a project resolves from a mirror or a private repository. Update-time then has nothing to measure staleness against. @@ -943,19 +943,25 @@ Maven Central does not report a withdrawal, so Update-time never warns that a Ma Update-time checks the pom's dependencies against OSV's Maven advisories. The version checked is the one the pom holds once Maven has run, so a vulnerability the run updated away from is never reported. -Update-time does not check a dependency when its group, artifact, or version names a property its parent declares. Update-time reads each pom on its own, so it leaves a property a parent declares as the pom wrote it. OSV matches an advisory to `groupId:artifactId` coordinates and a version, and a property name is neither of those. +Update-time does not check a dependency for vulnerabilities at all when its group, artifact, or version names a property its parent declares. Update-time does not resolve the properties a parent pom declares. OSV matches an advisory to `groupId:artifactId` coordinates and a version, and a property name is neither of those. Update-time does not check a dependency whose `` element is empty either, since OSV needs a version to match an advisory to. -Silencing this warning for a single dependency would need a marker, and Update-time does not yet support markers for Maven dependencies. Pass `--ignore-vulnerability` to silence an advisory across the run instead, or `--vulnerability-level` to only hear about the more severe ones. +Silencing the vulnerability warning for a single dependency would need a marker, and Update-time does not yet support markers for Maven dependencies. Pass `--ignore-vulnerability` to silence an advisory across the run instead, or `--vulnerability-level` to only hear about the more severe ones. #### Archived dependencies -Maven Central does not publish an archival signal of its own. It does serve the pom each version was published with. A pom's `` element may name the repository the project's source lives in. So Update-time reads that element. Where it names a GitHub repository, Update-time warns when GitHub reports that repository as archived. The pom's plugins are checked too. +Maven Central does not publish an archival signal of its own. So Update-time looks up the project's repository on GitHub, and warns when GitHub reports that repository as archived. It checks the pom's plugins too. -The pom read is the one beside the artefact's newest release, since archival is a fact about the project rather than about the version a reference pins. The repository is taken from the `` element's ``, ``, or ``, whichever of them names a GitHub repository first. +Update-time finds the repository in the pom that Maven Central serves beside the artefact's newest release. It reads the newest release's pom whatever version the reference pins, because archival is a fact about the project rather than about one of its versions. Update-time looks in these places, in this order, and takes the first one that names a GitHub repository: -Update-time warns only where it finds a GitHub repository to ask about. The pom it reads may not declare an `` element, as `com.google.guava:guava`'s does not. A project whose parent pom holds the `` reads the same way, since Update-time reads the artefact's own pom alone. An `` may name a host other than GitHub, as `org.apache.commons:commons-lang3` names `gitbox.apache.org`. And Maven Central may not serve a pom at all, for an artefact a project resolves from a mirror or a private repository. Update-time warns about a pom it cannot fetch or cannot parse, and does not take a repository from either. +1. The ``, ``, and `` of the `` element of the newest release's pom. That element names where the project's source lives. +2. The `` element of the newest release's pom, although it names the project's website more often than its repository. Update-time tries it after ``, because a module's `` may name the repository of the larger project the module is part of. The `` of `com.fasterxml.jackson.core:jackson-databind` names `FasterXML/jackson`, while its `` names `FasterXML/jackson-databind`. +3. The `` and `` of the parent pom that the `` element of the newest release's pom names. A project that consists of several modules often names its repository once, in the pom that all its modules inherit from. Update-time reads the parent pom only when the `` element of the newest release's pom does not name a URL. -Silencing this warning for a single dependency would need a marker, and Update-time does not yet support markers for Maven dependencies. Pass `--ignore-archived` to switch the check off for the whole run instead. +Update-time reads one parent pom, and does not read the parent pom's own parent. The poms higher up usually belong to an organisation rather than to a project. Every Apache project inherits from `org.apache:apache`, for example, which names the repository `apache/maven-apache-parent`. For the same reason, Update-time does not read the parent pom when the `` element of the newest release's pom names a repository outside GitHub. That parent is often an organisation's pom, too. + +Update-time warns only where it finds a GitHub repository to ask about. The newest release's pom and its parent pom may leave the repository unnamed. They may also name a host other than GitHub, as `org.apache.commons:commons-lang3` names `gitbox.apache.org`. And Maven Central may not serve a pom at all, for an artefact a project resolves from a mirror or a private repository. Update-time warns about a pom it cannot fetch or cannot parse. + +Silencing the archival warning for a single dependency would need a marker, and Update-time does not yet support markers for Maven dependencies. Pass `--ignore-archived` to switch the check off for the whole run instead. #### Markers diff --git a/justfile b/justfile index f8ae343..54c831d 100644 --- a/justfile +++ b/justfile @@ -290,7 +290,7 @@ check-readme-structure: # Check the readability of the prose in the code and the documentation. [private] check-readability: install-nltk-data - {{ start_capture() }} {{ python_m }} tools.readability_check --check-whitelist {{ code }} {{ prose }} {{ end_capture("check-readability") }} + {{ start_capture() }} {{ python_m }} tools.readability_check --check-whitelist {{ prose_whitelist }} {{ code }} {{ prose }} {{ end_capture("check-readability") }} # Run the quality checks. Run one by name for a quicker loop, e.g. `just ruff` or `just mypy`. [parallel] @@ -352,10 +352,11 @@ fix: install-py-dependencies # Regenerate the whitelists the checks read: the dead code vulture passes over, and the sentences the prose check passes over. update-whitelists: install-py-dependencies - # Every finding the checks report is written out, the ones this run introduced included, so read the diff. + # Vulture writes out every finding, the ones this run introduced included, so read the diff. The prose whitelist + # only drops the sentences the prose no longer holds, so a sentence that fails the check has to be rewritten. # Vulture returns exit code 3 when there is dead code, ignore it when writing the whitelist: {{ vulture }} --make-whitelist {{ code }} > {{ vulture_whitelist }} || true - {{ python_m }} tools.readability_check --make-whitelist {{ code }} {{ prose }} > {{ prose_whitelist }} + {{ python_m }} tools.readability_check --make-whitelist {{ prose_whitelist }} {{ code }} {{ prose }} # === Install dependencies === diff --git a/src/update_time/manifests/pom_xml.py b/src/update_time/manifests/pom_xml.py index e14c020..bc806bb 100644 --- a/src/update_time/manifests/pom_xml.py +++ b/src/update_time/manifests/pom_xml.py @@ -1,4 +1,4 @@ -"""Read the dependencies, properties, and source repository a pom.xml declares. +"""Read the dependencies, properties, parent, and source repository a pom.xml declares. This module owns what a pom's elements mean, whether Update-time scans the pom or a registry serves it. Reading the XML itself is the formats layer's concern. @@ -7,6 +7,7 @@ import re from typing import TYPE_CHECKING +from update_time.domain.dependency import PinnedDependency from update_time.domain.reference import Reference from update_time.formats import xml from update_time.primitives.location import Location @@ -29,22 +30,42 @@ # Update-time reads these children of ``, in this order, to find where the project's source lives. _SCM_URL_TAGS = ("url", "connection", "developerConnection") +# The children that name an artefact and its version, in the order `groupId:artifactId` names them. +_COORDINATE_TAGS = ("groupId", "artifactId", "version") -def scm_urls(document: bytes) -> list[str] | None: - """Return the URLs the pom's `` element names, `` first, or None when the pom's XML does not parse. + +def source_urls(project: XmlElement) -> list[str]: + """Return the URLs that may name where the project's source lives, the `` URLs first. + + The project's own `` comes after them, because a module's `` may name its umbrella project. + """ + urls = scm_urls(project) + project_url = project.child("url") + return urls if project_url is None else [*urls, project_url.text] + + +def scm_urls(project: XmlElement) -> list[str]: + """Return the URLs the pom's `` element names. Each is stripped of the `scm::` prefix Maven writes in front of it, leaving the URL that provider reads. """ - project = xml.parse(document) - if project is None: - return None scm = project.child("scm") - if scm is None: - return [] - named = (scm.child(tag) for tag in _SCM_URL_TAGS) + named = [] if scm is None else [scm.child(tag) for tag in _SCM_URL_TAGS] return [_SCM_PREFIX.sub("", url.text) for url in named if url is not None] +def parent(project: XmlElement) -> PinnedDependency | None: + """Return the artefact and version the pom's `` element names in full, or None where it does not.""" + element = project.child("parent") + if element is None: + return None + group_id, artifact_id, version = ( + "" if (child := element.child(tag)) is None else child.text for tag in _COORDINATE_TAGS + ) + pinned = PinnedDependency(_artefact(group_id, artifact_id), version) + return pinned if fully_resolved(pinned) else None + + def dependencies(path: Path) -> list[Reference] | None: """Return a reference to each dependency the pom declares, or None when the pom's XML does not parse. @@ -91,15 +112,18 @@ def _is_resolved(value: str) -> bool: return not _PROPERTY_REFERENCE.search(value) -def fully_resolved(reference: Reference) -> bool: - """Return whether the pom resolved the reference whole: its coordinates and the version it pins.""" - return _is_resolved(reference.dependency) and _is_resolved(reference.current_version) +def fully_resolved(pinned: PinnedDependency) -> bool: + """Return whether the pom named the pinned dependency whole: its group, its artifact, and its version. + + A part naming a property counts as unnamed, since this does not resolve the pom's properties. + """ + group_id, artifact_id = coordinates(pinned.name) + return all(part and _is_resolved(part) for part in (group_id, artifact_id, pinned.version)) def _own_coordinates(project: XmlElement) -> dict[str, XmlElement]: """Return the project's own coordinates, which a dependency on a sibling module names as `${project.groupId}`.""" - named = ("groupId", "artifactId", "version") - return {f"project.{tag}": element for tag in named if (element := project.child(tag)) is not None} + return {f"project.{tag}": element for tag in _COORDINATE_TAGS if (element := project.child(tag)) is not None} def properties(path: Path) -> dict[str, str]: @@ -119,6 +143,17 @@ def _property_elements(project: XmlElement) -> dict[str, XmlElement]: return anywhere | ({element.tag: element for element in own.children} if own else {}) +def coordinates(artefact: DependencyName) -> tuple[str, str]: + """Split an artefact's `groupId:artifactId` name into its group and its artifact.""" + group_id, _, artifact_id = artefact.partition(":") + return group_id, artifact_id + + +def _artefact(group_id: str, artifact_id: str) -> DependencyName: + """Join a group and an artifact into the artefact's `groupId:artifactId` name.""" + return f"{group_id}:{artifact_id}" + + def _reference( path: Path, element: XmlElement, property_elements: dict[str, XmlElement], default_group: str = "" ) -> Reference | None: @@ -131,11 +166,12 @@ def _reference( artifact = element.child("artifactId") version = element.child("version") group_name = default_group if group is None else _resolved(group, property_elements).text - if not group_name or artifact is None or version is None: + artifact_name = "" if artifact is None else _resolved(artifact, property_elements).text + if not group_name or not artifact_name or version is None: return None versioned_by = _resolved(version, property_elements) location = Location(path, versioned_by.line, versioned_by.column) - return Reference(f"{group_name}:{_resolved(artifact, property_elements).text}", versioned_by.text, location) + return Reference(_artefact(group_name, artifact_name), versioned_by.text, location) def _resolved(element: XmlElement, property_elements: dict[str, XmlElement]) -> XmlElement: diff --git a/src/update_time/package_managers/maven.py b/src/update_time/package_managers/maven.py index 1ccc113..b7ffc3c 100644 --- a/src/update_time/package_managers/maven.py +++ b/src/update_time/package_managers/maven.py @@ -117,7 +117,7 @@ def _rule(artefact: DependencyName, versions: tuple[str, ...]) -> str: An `ignoreVersion` without a `type` attribute matches a version exactly, so a version is never read as a pattern. """ - group_id, _, artifact_id = artefact.partition(":") + group_id, artifact_id = pom_xml_format.coordinates(artefact) ignored = "".join(f" {version}\n" for version in versions) return ( f' \n' diff --git a/src/update_time/sources/github.py b/src/update_time/sources/github.py index d76feb8..55dd16d 100644 --- a/src/update_time/sources/github.py +++ b/src/update_time/sources/github.py @@ -270,21 +270,24 @@ def github_to_raw(url: str) -> str: # Matches `git@github.com:` in `git@github.com:owner/repo.git`, capturing the user and host. _SCP_LIKE_RE = re.compile(r"^([^/@]+@[^/:]+):") +# GitHub serves its sponsorship pages under this path, which it reserves, so no owner can go by this name. +_GITHUB_SPONSORS_PATH = "sponsors" def github_owner_and_repository(url: str) -> tuple[str, str]: """Parse the GitHub owner and repository from a URL. Accepts npm-style `git+https`, `git+ssh`, and `.git` URLs, plus git's scp-like `git@github.com:owner/repo` form, - which is rewritten to an ssh URL so its host is read the same way as every other form's. + which is rewritten to an ssh URL so its host is read the same way as every other form's. A `github.com/sponsors/…` + URL names a sponsorship page rather than a repository, so it parses as none. """ normalized_url = _SCP_LIKE_RE.sub(r"ssh://\1/", url.removeprefix("git+")) parsed = urlparse(normalized_url) if parsed.hostname == "github.com": path_parts = parsed.path.lstrip("/").split("/") - if len(path_parts) > 1: + if len(path_parts) > 1 and path_parts[0] != _GITHUB_SPONSORS_PATH: return path_parts[0], path_parts[1].removesuffix(".git") - return NO_CHANGES, "" + return "", "" def _owner_and_repository(dependency: DependencyName) -> tuple[str, str]: @@ -530,10 +533,19 @@ def _newest_release(owner: str, repository: str) -> Release | None: ) -def _get_release(owner: str, repository: str, package: str, version: str) -> TaggedVersion | None: - """Get the release matching the package and version from the GitHub releases API. +def _get_release(owner: str, repository: str, tags: list[str]) -> TaggedVersion | None: + """Get the release carrying the first of the tags that the repository released under.""" + releases_by_tag = {release["tag_name"]: release for release in (_list_releases(owner, repository) or ())} + for tag in tags: + if tag in releases_by_tag: + return TaggedVersion.from_release(owner, repository, releases_by_tag[tag]) + return None + + +def release_tags(package: str, version: str, *aliases: str) -> list[str]: + """Return the tags a repository may release the package's version under, in order of preference. - Tries tag names in order of preference, repeating the first four for each name `_package_names` returns: + The first four repeat for each name `_package_names` returns, and then for each alias: 1. `-v` (monorepo, e.g. `puppeteer-core-v25.0.4`). 2. `-` (monorepo without the `v`, e.g. `selenium-4.47.0`). 3. `@` (monorepo joining the two with an `@`, e.g. `astro@7.1.4`). @@ -541,12 +553,9 @@ def _get_release(owner: str, repository: str, package: str, version: str) -> Tag 5. `v` (e.g. `v25.0.4`). 6. `` (e.g. `25.0.4`). """ - releases_by_tag = {release["tag_name"]: release for release in (_list_releases(owner, repository) or ())} - package_tags = [f"{name}{joiner}{version}" for name in _package_names(package) for joiner in ("-v", "-", "@", "/")] - for tag in [*package_tags, f"v{version}", version]: - if tag in releases_by_tag: - return TaggedVersion.from_release(owner, repository, releases_by_tag[tag]) - return None + names = [*_package_names(package), *aliases] + package_tags = [f"{name}{joiner}{version}" for name in names for joiner in ("-v", "-", "@", "/")] + return [*package_tags, f"v{version}", version] def _package_names(package: str) -> list[str]: @@ -562,10 +571,15 @@ def _package_names(package: str) -> list[str]: def changes_from_release(owner: str, repository: str, package: str, version: str) -> Changes: - """Return the body of the GitHub release matching the package and version, or empty string if absent.""" + """Return the body of the GitHub release matching the package and version.""" + return changes_from_tagged_release(owner, repository, release_tags(package, version)) + + +def changes_from_tagged_release(owner: str, repository: str, tags: list[str]) -> Changes: + """Return the body of the GitHub release carrying the first of the tags.""" if not (owner and repository): return NO_CHANGES - release = _get_release(owner, repository, package, version) + release = _get_release(owner, repository, tags) return release.body if release else NO_CHANGES diff --git a/src/update_time/sources/maven_central.py b/src/update_time/sources/maven_central.py index 9b2ae7c..3ed79ee 100644 --- a/src/update_time/sources/maven_central.py +++ b/src/update_time/sources/maven_central.py @@ -13,15 +13,22 @@ from update_time.domain.archival import archival_reporting from update_time.domain.cooldown import within_cooldown -from update_time.domain.dependency import Archival, Project, Release +from update_time.domain.dependency import NO_CHANGES, Archival, Changes, Project, Release +from update_time.formats import xml from update_time.io.fetch import fetch from update_time.io.log import get_logger from update_time.manifests import pom_xml as pom_xml_format from update_time.sources.github import archival as github_archival -from update_time.sources.github import github_owner_and_repository +from update_time.sources.github import ( + changes_from_changelog_file, + changes_from_tagged_release, + github_owner_and_repository, + release_tags, +) if TYPE_CHECKING: from update_time.domain.dependency import DependencyName, VersionString + from update_time.formats.xml import XmlElement _LOG = get_logger("maven central") @@ -39,59 +46,127 @@ def project(artefact: DependencyName, *, check_archival: bool) -> Project: """Return the artefact's newest release on the repository, with the archival GitHub declares for its source.""" newest = _newest_release(artefact) - return Project(newest=newest, archival=_archival(artefact, newest) if check_archival else Archival()) - + return Project(newest=newest, archival=_archival(artefact) if check_archival else Archival()) -def _archival(artefact: DependencyName, newest: Release | None) -> Archival: - """Return what GitHub declares about the repository the artefact's pom names. - The pom read is the one beside the newest release, since archival is a fact about the project. A project that - moved to GitHub names the repository in its later poms alone. Update-time can read a pom only for an artefact it - found a dated version of. - """ - if newest is None: - return Archival() - owner, repository = _scm_repository(artefact, newest.version) +def _archival(artefact: DependencyName) -> Archival: + """Return what GitHub declares about the artefact's repository.""" + owner, repository = _repository(artefact) if not repository: return Archival() return github_archival(owner, repository, check_archival=True) -# What Update-time reads for a pom that does not name a repository on GitHub. +def get_changes(artefact: DependencyName, version: VersionString) -> Changes: + """Return the version's changes, from the release notes the artefact's GitHub repository published for it. + + The changes come from the repository's changelog file when it did not publish a release for the version. + """ + owner, repository = _repository(artefact) + tags = _release_tags(artefact, repository, version) + return changes_from_tagged_release(owner, repository, tags) or _changes_from_changelog_file( + owner, repository, version + ) + + +# The qualifier Maven appends to a version, such as `-jre` or `.Final`, which a repository may leave out of its tags. +_QUALIFIER = re.compile(r"[.-][A-Za-z].*$") +# A Maven project may prefix a version's tag with one of these instead of a `v`, as JUnit tags `r6.1.3`. +_VERSION_PREFIXES = ("r", "version-", "REL") + + +def _spellings(version: VersionString) -> tuple[str, str]: + """Return the spellings a repository may give the version: as Maven spells it, and without its qualifier.""" + return (version, _QUALIFIER.sub("", version)) + + +def _release_tags(artefact: DependencyName, repository: str, version: VersionString) -> list[str]: + """Return the tags the artefact's repository may release the version under, the version as Maven spells it first. + + A repository may tag a release by the artifact's name, without its group, or by its own name. + """ + _group_id, artifact_id = pom_xml_format.coordinates(artefact) + tags = ( + tag + for spelling in _spellings(version) + for tag in [ + *release_tags(artifact_id, spelling, repository), + *(f"{prefix}{spelling}" for prefix in _VERSION_PREFIXES), + ] + ) + return list(dict.fromkeys(tags)) + + +def _changes_from_changelog_file(owner: str, repository: str, version: VersionString) -> Changes: + """Return the version's changes from the repository's changelog file, the version as Maven spells it first.""" + for spelling in _spellings(version): + if changes := changes_from_changelog_file(owner, repository, spelling): + return changes + return NO_CHANGES + + +# What Update-time reads for an artefact where it does not find a repository on GitHub. _NO_REPOSITORY = ("", "") -def _scm_repository(artefact: DependencyName, version: VersionString) -> tuple[str, str]: - """Return the owner and repository the version's pom names in its ``, empty where it names none on GitHub.""" - document = _pom(artefact, version) - if document is None: +def _repository(artefact: DependencyName) -> tuple[str, str]: + """Return the owner and repository of the artefact's project, read from the pom beside its newest release. + + Where the project's source lives is a fact about the project rather than about a version. A project that moved to + GitHub names the repository in its later poms alone. Update-time can read a pom only for an artefact it found a + dated version of. + """ + newest = _newest_release(artefact) + return _NO_REPOSITORY if newest is None else _pom_repository(artefact, newest.version) + + +def _pom_repository(artefact: DependencyName, version: VersionString) -> tuple[str, str]: + """Return the owner and repository the version's pom names, or else the one its parent pom names.""" + pom = _pom(artefact, version) + if pom is None: return _NO_REPOSITORY - scm_urls = pom_xml_format.scm_urls(document) - if scm_urls is None: - _LOG.invalid_pom(_pom_url(artefact, version)) + named = _named_repository(pom) + if named != _NO_REPOSITORY: + return named + return _named_repository(_parent_pom(pom)) + + +def _parent_pom(pom: XmlElement) -> XmlElement | None: + """Return the parent pom to read the repository from, or None where the pom's `` names a URL of its own.""" + if pom_xml_format.scm_urls(pom): + return None + parent = pom_xml_format.parent(pom) + return None if parent is None else _pom(parent.name, parent.version) + + +def _named_repository(pom: XmlElement | None) -> tuple[str, str]: + """Return the owner and repository the pom names, empty where it names none on GitHub.""" + if pom is None: return _NO_REPOSITORY - for scm_url in scm_urls: - owner, repository = github_owner_and_repository(scm_url) + for url in pom_xml_format.source_urls(pom): + owner, repository = github_owner_and_repository(url) if owner and repository: return owner, repository return _NO_REPOSITORY def _pom_url(artefact: DependencyName, version: VersionString) -> str: - """Return the URL of the pom the repository serves beside the artefact's version. - - The repository names a pom after the artefact, which its directory is named after too. - """ - artefact_url = _artefact_url(artefact) - artifact_id = artefact_url.rpartition("/")[2] - return f"{artefact_url}/{version}/{artifact_id}-{version}.pom" + """Return the URL of the pom the repository serves beside the artefact's version, named after the artifact.""" + _group_id, artifact_id = pom_xml_format.coordinates(artefact) + return f"{_artefact_url(artefact)}/{version}/{artifact_id}-{version}.pom" @cache -def _pom(artefact: DependencyName, version: VersionString) -> bytes | None: - """Return the pom the repository serves beside the artefact's version, or None where fetching it failed.""" - response = fetch(_pom_url(artefact, version), _LOG) - return None if response is None else response.content +def _pom(artefact: DependencyName, version: VersionString) -> XmlElement | None: + """Return the pom served beside the artefact's version, or None where it is unserved or unparsable.""" + url = _pom_url(artefact, version) + response = fetch(url, _LOG) + if response is None: + return None + pom = xml.parse(response.content) + if pom is None: + _LOG.invalid_pom(url) + return pom def _newest_release(artefact: DependencyName) -> Release | None: @@ -117,7 +192,7 @@ def _artefact_url(artefact: DependencyName) -> str: Maven names an artefact `groupId:artifactId`, and serves it under the group's dots spelled as directories. """ - group_id, _, artifact_id = artefact.partition(":") + group_id, artifact_id = pom_xml_format.coordinates(artefact) return f"{_MAVEN_CENTRAL}/{group_id.replace('.', '/')}/{artifact_id}" diff --git a/src/update_time/sources/pypi.py b/src/update_time/sources/pypi.py index 435e8a2..d83653d 100644 --- a/src/update_time/sources/pypi.py +++ b/src/update_time/sources/pypi.py @@ -60,8 +60,6 @@ # with its aliases, then its `homepage` label. All spelled as `_normalized_label` returns them. _REPOSITORY_URL_LABELS_BY_RANK = ({"source", "repository", "sourcecode", "github"}, {"homepage"}) _LABEL_NORMALIZATION = str.maketrans("", "", string.punctuation + string.whitespace) -# GitHub serves its sponsorship pages under this path, which it reserves, so no owner can go by this name. -_GITHUB_SPONSORS_PATH = "sponsors" # Matches a GitHub repository URL wherever it sits in prose, such as the description a project posts to PyPI. _GITHUB_URL_RE = re.compile(r"https://github\.com/[\w.-]+/[\w.-]+") @@ -357,27 +355,17 @@ def _changelog_from_github_url_in_description(description: str, package: str, ve def _names_the_package(url: str, package: str) -> bool: """Return whether the URL points at a repository carrying the package's name.""" - _owner, repository = _github_repository(url) + _owner, repository = github_owner_and_repository(url) return normalized_python_name(repository) == normalized_python_name(package) -def _github_repository(url: str) -> tuple[str, str]: - """Return the owner and repository the URL points at, or empty strings when it points at no repository. - - A `github.com/sponsors/…` URL parses as the repository `sponsors/`, which does not exist, so it is - reported as no repository rather than queried. - """ - owner, repository = github_owner_and_repository(url) - return ("", "") if owner == _GITHUB_SPONSORS_PATH else (owner, repository) - - def _changelog_from_github_releases(url: str, package: str, version: str) -> Changes: """Get the changelog from the GitHub releases.""" - owner, repository = _github_repository(url) + owner, repository = github_owner_and_repository(url) return changes_from_release(owner, repository, package, version) def _changelog_from_repository_root(url: str, version: str) -> Changes: """Get the changelog from a changelog file in the root of the repository.""" - owner, repository = _github_repository(url) + owner, repository = github_owner_and_repository(url) return changes_from_changelog_file(owner, repository, version) diff --git a/src/update_time/updaters/update_pom_xml.py b/src/update_time/updaters/update_pom_xml.py index e31a4bc..7b52e7f 100644 --- a/src/update_time/updaters/update_pom_xml.py +++ b/src/update_time/updaters/update_pom_xml.py @@ -2,7 +2,7 @@ from typing import TYPE_CHECKING -from update_time.domain.dependency import DependencyVersion +from update_time.domain.dependency import NO_CHANGES, DependencyVersion from update_time.domain.file_type import POM_XML from update_time.domain.reference import Reference from update_time.formats import xml @@ -64,18 +64,20 @@ def _warn_about_vulnerabilities(declared: list[Reference]) -> None: steered = [ SteeredReference.from_reference(declaration) for declaration in declared - if pom_xml_format.fully_resolved(declaration) + if pom_xml_format.fully_resolved(declaration.pinned) ] warn_about_vulnerable_dependencies([steered], Ecosystem.MAVEN, _LOG) def _report_new_versions(before: list[Reference], after: list[Reference]) -> None: - """Report each dependency whose version differs between the two readings.""" + """Report each dependency whose version differs between the two readings, with the new version's changes.""" for old, new in zip(before, after, strict=True): if old.current_version == new.current_version: continue updated = Reference(new.dependency, old.current_version, new.location) - _LOG.new_version(updated, DependencyVersion(new.current_version)) + resolved = pom_xml_format.fully_resolved(new.pinned) + changes = maven_central.get_changes(new.dependency, new.current_version) if resolved else NO_CHANGES + _LOG.new_version(updated, DependencyVersion(new.current_version, changes)) def main() -> None: # pragma: no cover diff --git a/tests/mutation-survivals.md b/tests/mutation-survivals.md index 338e69c..bdd5068 100644 --- a/tests/mutation-survivals.md +++ b/tests/mutation-survivals.md @@ -14,3 +14,6 @@ - 2026-09-13 `tests.update_time.io.test_log.RecordRenderingTests.test_the_markdown_parser_logs_nothing_at_debug_level` — every record without changes gets a blank line below it - 2026-09-14 `tests.update_time.io.test_console.RecordRenderingTests.test_a_record_without_changes_gets_no_empty_block` — every record without changes gets a blank line below it - 2026-09-14 `tests.update_time.io.test_console.RecordRenderingTests.test_a_note_about_the_changelog_is_not_boxed` — Update-time's own note about a changelog is boxed as if it were a changelog's changes +- 2026-09-24 `tests.update_time.sources.test_maven_central.ProjectTest.test_an_artefact_is_asked_for_its_pom_once_per_run` — every pom declaring an artefact costs a pom request of its own +- 2026-09-25 `tests.update_time.sources.test_maven_central.GetChangesTest.test_a_release_tagged_with_the_artifact_id_matches` — a release tagged by the artifact's name goes unmatched, since the tag names never carry the group +- 2026-09-26 `tests.update_time.sources.test_maven_central.ProjectTest.test_a_parent_pom_naming_no_repository_leaves_its_own_parent_unread` — an artefact's grandparent pom is read too, which may name a generic parent project's repository diff --git a/tests/mutation.py b/tests/mutation.py index c10b936..59edf7c 100644 --- a/tests/mutation.py +++ b/tests/mutation.py @@ -21,7 +21,7 @@ from unittest.mock import patch if TYPE_CHECKING: - from collections.abc import Callable + from collections.abc import Callable, Iterator # Holds the id of the test being re-run against its own mutation. That test must run its body and stop there: # checking its mutations again would check them against themselves, without end. Only the test it names stands @@ -58,7 +58,7 @@ class Outcome(StrEnum): KILLED and SURVIVED judge the test: it failed against the mutation, or it did not. STALE and BROKEN judge the mutation instead, and call for rewriting the mutation rather than the test. Stale means it was never applied: reading or parsing the file failed, the source lacks the anchor, the anchor holds the snippet other than once, - or the replacement equals the snippet. + the replacement equals the snippet, or the anchor is a module while one function holds the snippet. Broken means the mutated source did not import, or the test errored with an error other than the one the mutation declares. """ @@ -92,18 +92,20 @@ class Mutation: scaffolding. Naming the error in `raises` tells the two apart: the test kills the mutation by raising that error, while any other error is reported as broken. - The anchor names the code to change: a module, or a function, method or property the module holds. The - qualified name reaches a member through its class, and only the source of the definition is changed. + The anchor names the code to change: a module, or a class, function, method, classmethod or property the module + holds. The qualified name reaches a member through its class, and only the source of the definition is changed. + A module anchor is for a snippet that spans several definitions or lies outside them. A module anchor is stale + when one function or method holds the snippet. """ - anchor: types.ModuleType | _Function | property + anchor: types.ModuleType | _Function | types.MethodType | property old: str new: str regression: str = "" raises: str = "" @property - def _unwrapped(self) -> types.ModuleType | _Function: + def _unwrapped(self) -> types.ModuleType | _Function | types.MethodType: """Return the anchor in the form that carries its names, which for a property is the getter behind it.""" return cast("_Function", self.anchor.fget) if isinstance(self.anchor, property) else self.anchor @@ -140,7 +142,11 @@ def _mutated(self) -> str: source = Path(self._path).read_text() except OSError as error: raise StaleError(_reason(error)) from error - return mutated_source(source, self._qualified_name, self.old, self.new) + mutated = mutated_source(source, self._qualified_name, self.old, self.new) + if not self._qualified_name and (function := _function_holding(source, self.old)): + message = f"the snippet lies inside {function}, so anchor the mutation on {function}" + raise StaleError(message) + return mutated def check(self, test_name: str) -> Result: """Return what checking the mutation showed: whether the test fails against it, or the file has moved on. @@ -216,32 +222,53 @@ def _span(source: str, qualified_name: str) -> tuple[int, int]: """Return the offsets of the part of the source a mutation may change, which is all of it for an empty name.""" if not qualified_name: return 0, len(source) + definition = _definitions(source).get(qualified_name) + if definition is None: + message = f"the source does not define {qualified_name}" + raise StaleError(message) + return _node_span(source, definition) + + +def _function_holding(source: str, snippet: str) -> str: + """Return the qualified name of the function or method whose span holds the snippet, or the empty string.""" + start = source.index(snippet) + end = start + len(snippet) + functions = {name: node for name, node in _definitions(source).items() if not isinstance(node, ast.ClassDef)} + spans = {name: _node_span(source, node) for name, node in functions.items()} + return next((name for name, (first, last) in spans.items() if first <= start and end <= last), "") + + +def _definitions(source: str) -> dict[str, ast.stmt]: + """Return each class, function and method the source defines, under the qualified name an anchor reaches it by. + + A name defined twice, as a property's getter and setter are, names its first definition, which is the getter. + """ try: body = ast.parse(source).body except SyntaxError as error: raise StaleError(_reason(error)) from error - definition = _definition(body, qualified_name.split(".")) - if definition is None: - message = f"the source does not define {qualified_name}" - raise StaleError(message) - start = _offset(source, definition.lineno, definition.col_offset) - return start, _offset(source, definition.end_lineno or 0, definition.end_col_offset or 0) - - -def _definition(body: list[ast.stmt], qualified_name: list[str]) -> ast.stmt | None: - """Return the definition the qualified name points at, or None where the source does not define it.""" - name, *rest = qualified_name - found = next( - ( - node - for node in body - if isinstance(node, (ast.FunctionDef, ast.AsyncFunctionDef, ast.ClassDef)) and node.name == name - ), - None, - ) - if found is None: - return None - return _definition(found.body, rest) if rest else found + definitions: dict[str, ast.stmt] = {} + for name, node in _named(body): + definitions.setdefault(name, node) + return definitions + + +def _named(body: list[ast.stmt], prefix: str = "") -> Iterator[tuple[str, ast.stmt]]: + """Yield each class, function and method the body defines, recursing into classes but not into functions.""" + for node in body: + if isinstance(node, (ast.ClassDef, ast.FunctionDef, ast.AsyncFunctionDef)): + yield f"{prefix}{node.name}", node + if isinstance(node, ast.ClassDef): + yield from _named(node.body, f"{prefix}{node.name}.") + + +def _node_span(source: str, node: ast.stmt) -> tuple[int, int]: + """Return the offsets in the source where the node starts and ends, the newline ending its last line included.""" + start = _offset(source, node.lineno, node.col_offset) + end = _offset(source, node.end_lineno or 0, node.end_col_offset or 0) + if source.startswith("\n", end): + end += 1 + return start, end def _offset(source: str, line: int, column: int) -> int: diff --git a/tests/mutation_subject.py b/tests/mutation_subject.py index d8c40dd..208d3c7 100644 --- a/tests/mutation_subject.py +++ b/tests/mutation_subject.py @@ -6,6 +6,15 @@ def is_even(number: int) -> bool: return number % 2 == 0 +def is_positive_even(count: int) -> bool: + """Return whether the count is positive and even, where a nested function decides whether it is positive.""" + + def positive() -> bool: + return count > 0 + + return positive() and is_even(count) + + def is_multiple_of_three(value: int) -> bool: """Return whether the value is a multiple of three.""" return value % 3 == 0 @@ -14,7 +23,7 @@ def is_multiple_of_three(value: int) -> bool: class Doubler: """A value doubler whose two methods end on the same snippet, so an anchor has to reach the method itself. - Its property is here for an anchor to name, a property being neither a plain function nor a module. + Its property and its classmethod are here for an anchor to name, neither being a plain function nor a module. """ def doubled(self, value: int) -> int: @@ -29,3 +38,8 @@ def quadrupled(self, value: int) -> int: def one_doubled(self) -> int: """Return one doubled.""" return self.doubled(1) + + @classmethod + def two_doubled(cls) -> int: + """Return two doubled.""" + return cls().doubled(2) diff --git a/tests/test_mutation.py b/tests/test_mutation.py index 6914bec..c0b0b67 100644 --- a/tests/test_mutation.py +++ b/tests/test_mutation.py @@ -19,7 +19,7 @@ from tests import mutation as checker from tests.helpers import patch_environ from tests.mutation import CHECKED_TEST, CHECKS_OFF, Mutation, Outcome, Result, _failure, _record_survival, kills -from tests.mutation_subject import Doubler, is_even, is_multiple_of_three +from tests.mutation_subject import Doubler, is_even, is_multiple_of_three, is_positive_even if TYPE_CHECKING: from collections.abc import Iterator @@ -36,6 +36,7 @@ _MULTIPLE_TEST_NAME = "tests.test_mutation.IsMultipleOfThreeTest.test_a_multiple_of_three" _DOUBLER_TEST_NAME = "tests.test_mutation.DoublerTest.test_a_doubled_value" _ONE_DOUBLED_TEST_NAME = "tests.test_mutation.DoublerTest.test_one_doubled" +_TWO_DOUBLED_TEST_NAME = "tests.test_mutation.DoublerTest.test_two_doubled" # The `IsEvenTest` tests `KillsTest` runs to exercise the decorator: one registers a single mutation, one several. _DECORATED_TEST = "test_an_odd_number" _SEVERAL_MUTATIONS_TEST = "test_an_odd_number_against_several_mutations" @@ -48,8 +49,21 @@ _NAME_ERROR = "NameError: name 'nonexistent' is not defined" _UNPARSABLE = "number %" # A replacement that leaves the subject unparsable, so it does not import at all. _SURVIVING = "number == 2" # A replacement the test passes against, so nothing it asserts breaks. -_ODD_REPORTED_AS_EVEN = Mutation(mutation_subject, _EVEN, _ODD, _REGRESSION) -_ONLY_THREE_REPORTED_AS_EVEN = Mutation(mutation_subject, _EVEN, _ONLY_THREE, _ONLY_THREE_REGRESSION) +_ODD_REPORTED_AS_EVEN = Mutation(mutation_subject.is_even, _EVEN, _ODD, _REGRESSION) +_ONLY_THREE_REPORTED_AS_EVEN = Mutation(mutation_subject.is_even, _EVEN, _ONLY_THREE, _ONLY_THREE_REGRESSION) +# The regressions of the walk over a source's definitions that both an anchor and the anchor check rely on. +_CLASSES_SKIPPED = Mutation( + checker._named, + ' yield from _named(node.body, f"{prefix}{node.name}.")', + " pass", + "the walk skips classes, so a method is neither found for its anchor nor seen by the check", +) +_SPAN_STOPS_BEFORE_THE_NEWLINE = Mutation( + checker._node_span, + ' if source.startswith("\\n", end):\n end += 1\n', + "", + "a span stops before the newline ending a definition's last line, so a snippet ending in it escapes both", +) class IsEvenTest(unittest.TestCase): @@ -63,6 +77,10 @@ def test_an_even_number(self): """ self.assertTrue(is_even(2)) + def test_a_positive_even_number(self): + """Test that two is positive and even.""" + self.assertTrue(is_positive_even(2)) + @kills(_ODD_REPORTED_AS_EVEN) def test_an_odd_number(self): """Test that an odd number is not even.""" @@ -98,7 +116,7 @@ def test_an_odd_number_against_several_mutations(self): """Test that an odd number is not even, killing each of the mutations its registration holds.""" self.assertFalse(is_even(3)) - @kills(Mutation(mutation_subject, _EVEN, _ERRORING, _RAISING_REGRESSION, raises=_NAME_ERROR)) + @kills(Mutation(mutation_subject.is_even, _EVEN, _ERRORING, _RAISING_REGRESSION, raises=_NAME_ERROR)) def test_an_odd_number_against_a_mutation_that_raises(self): """Test that an odd number is not even, killing a mutation by raising the error the mutation declares.""" self.assertFalse(is_even(3)) @@ -127,29 +145,26 @@ def test_one_doubled(self): """Test that one doubled is two.""" self.assertEqual(Doubler().one_doubled, 2) + def test_two_doubled(self): + """Test that two doubled is four.""" + self.assertEqual(Doubler.two_doubled(), 4) + class CheckTest(unittest.TestCase): """Unit tests for checking a test against the mutation it is meant to kill.""" - def test_a_test_that_fails_against_the_mutation(self): - """Test that a mutation the registered test fails against is reported as killed.""" - self.assertEqual(Mutation(mutation_subject, _EVEN, _ODD).check(_SUBJECT_TEST_NAME), Result(Outcome.KILLED)) - + @kills(_SPAN_STOPS_BEFORE_THE_NEWLINE) def test_a_mutation_anchored_to_a_function_is_applied_inside_it(self): - """Test that a mutation anchored to a function is applied to it, so its test is reported as killed.""" + """Test that a mutation anchored to a function is applied to it, up to the newline after its last line.""" self.assertEqual(Mutation(is_even, _EVEN, _ODD).check(_SUBJECT_TEST_NAME), Result(Outcome.KILLED)) + self.assertEqual(Mutation(is_even, f"{_EVEN}\n", f"{_ODD}\n").check(_SUBJECT_TEST_NAME), Result(Outcome.KILLED)) @kills( - Mutation( - checker, - " return _definition(found.body, rest) if rest else found", - " return found", - "the walk stops at the class, so the change may land in a sibling method", - ), + _CLASSES_SKIPPED, Mutation( checker._span, - 'qualified_name.split(".")', - 'qualified_name.split(".")[-1:]', + "(qualified_name)", + '(qualified_name.split(".")[-1])', "the lookup uses the bare name, so a method is searched for at the top of the module and never found", ), ) @@ -175,6 +190,24 @@ def test_a_mutation_anchored_to_a_property_is_applied_inside_its_getter(self): mutation = Mutation(Doubler.one_doubled, "self.doubled(1)", "self.doubled(2)") self.assertEqual(mutation.check(_ONE_DOUBLED_TEST_NAME), Result(Outcome.KILLED)) + @kills( + Mutation( + checker._named, + " if isinstance(node, (ast.ClassDef, ast.FunctionDef, ast.AsyncFunctionDef)):", + " if isinstance(node, (ast.FunctionDef, ast.AsyncFunctionDef)):", + "a class is left out of the definitions, so a mutation anchored to one is reported stale", + ) + ) + def test_a_mutation_anchored_to_a_class_is_applied_inside_it(self): + """Test that a mutation anchored to a class is applied to a snippet in its body, outside its methods.""" + mutation = Mutation(Doubler, "A value doubler", "A doubler") + self.assertEqual(mutation.check(_DOUBLER_TEST_NAME), Result(Outcome.SURVIVED)) + + def test_a_mutation_anchored_to_a_classmethod_is_applied_inside_it(self): + """Test that a mutation anchored to a classmethod is applied to it, so its test is reported as killed.""" + mutation = Mutation(Doubler.two_doubled, "cls().doubled(2)", "cls().doubled(3)") + self.assertEqual(mutation.check(_TWO_DOUBLED_TEST_NAME), Result(Outcome.KILLED)) + def test_a_snippet_the_anchored_function_holds_once_and_the_file_twice(self): """Test that a snippet the anchored function holds once is applied there, and nowhere else in the file.""" ambiguous = Result(Outcome.STALE, "the snippet occurs 2 times rather than once") @@ -187,7 +220,7 @@ def test_a_snippet_the_anchored_function_holds_once_and_the_file_twice(self): @kills( Mutation( - checker, + checker.Mutation.check, " return Result(Outcome.KILLED if test_result.failures else Outcome.SURVIVED)", " return Result(Outcome.KILLED if test_result.failures or self.raises else Outcome.SURVIVED)", "a declaration alone counts as a kill, so a mutation the test passes against is reported as killed", @@ -197,7 +230,7 @@ def test_a_test_that_passes_against_the_mutation(self): """Test that a mutation the registered test passes against is reported as survived, declared error or not.""" for case, declared in (("nothing declared", ""), ("an error declared", _NAME_ERROR)): with self.subTest(case=case): - mutation = Mutation(mutation_subject, _EVEN, _SURVIVING, raises=declared) + mutation = Mutation(mutation_subject.is_even, _EVEN, _SURVIVING, raises=declared) self.assertEqual(mutation.check(_SUBJECT_TEST_NAME), Result(Outcome.SURVIVED)) def test_a_mutation_whose_file_and_test_are_in_different_packages(self): @@ -208,7 +241,7 @@ def test_a_mutation_whose_file_and_test_are_in_different_packages(self): """ importlib.import_module("update_time.domain.staleness") mutation = Mutation( - timestamp, + timestamp.days_since, "return (datetime.now(UTC) - timestamp).days", "return (datetime.now(UTC) - timestamp).days + 1", ) @@ -217,14 +250,14 @@ def test_a_mutation_whose_file_and_test_are_in_different_packages(self): @kills( Mutation( - checker, + checker._loaded_test, " return unittest.TestLoader().loadTestsFromName(test_name)\n except Exception as error:", " return unittest.TestLoader().loadTestsFromName(test_name)\n except SyntaxError as error:", "a mutation the test module cannot import escapes the check rather than being reported as broken", raises="AttributeError: 'function' object has no attribute '_registered_mutations'", ), Mutation( - checker, + checker._loaded_test, " return unittest.TestLoader().loadTestsFromName(test_name)\n" " except Exception as error:\n" " raise _SourceError(_reason(error)) from error", @@ -237,13 +270,17 @@ def test_a_mutation_whose_file_and_test_are_in_different_packages(self): def test_a_mutation_that_cannot_be_judged_is_reported_as_broken(self): """Test that a mutation the check could not judge is reported as broken rather than as survived.""" deletes_the_registration = Mutation( - checker, + checker.kills, " setattr(wrapper, _REGISTERED, mutations)", " delattr(wrapper, _REGISTERED)", ) for case, mutation, error in ( - ("the mutated source does not import", Mutation(mutation_subject, _EVEN, _UNPARSABLE), "SyntaxError"), - ("the test errors", Mutation(mutation_subject, _EVEN, _ERRORING), "NameError"), + ( + "the mutated source does not import", + Mutation(mutation_subject.is_even, _EVEN, _UNPARSABLE), + "SyntaxError", + ), + ("the test errors", Mutation(mutation_subject.is_even, _EVEN, _ERRORING), "NameError"), ("the test module raises when imported", deletes_the_registration, "AttributeError"), ): with self.subTest(case=case): @@ -253,7 +290,7 @@ def test_a_mutation_that_cannot_be_judged_is_reported_as_broken(self): @kills( Mutation( - checker, + checker.Mutation.check, " return Result(Outcome.BROKEN, str(error))", " return Result(Outcome.KILLED if str(error) == self.raises else Outcome.BROKEN, str(error))", "a mutation whose source does not import counts as killed where the declaration names the import's error", @@ -261,13 +298,13 @@ def test_a_mutation_that_cannot_be_judged_is_reported_as_broken(self): ) def test_a_source_that_does_not_import_though_its_error_is_declared(self): """Test that a mutation whose source does not import is reported as broken though it declares that error.""" - reported = Mutation(mutation_subject, _EVEN, _UNPARSABLE).check(_SUBJECT_TEST_NAME).reason - declaring = Mutation(mutation_subject, _EVEN, _UNPARSABLE, raises=reported) + reported = Mutation(mutation_subject.is_even, _EVEN, _UNPARSABLE).check(_SUBJECT_TEST_NAME).reason + declaring = Mutation(mutation_subject.is_even, _EVEN, _UNPARSABLE, raises=reported) self.assertEqual(declaring.check(_SUBJECT_TEST_NAME), Result(Outcome.BROKEN, reported)) @kills( Mutation( - checker, + checker.Mutation.check, "return Result(Outcome.KILLED) if raised == self.raises else Result(Outcome.BROKEN, raised)", "return Result(Outcome.BROKEN, raised)", "a test that kills a mutation by raising the error the mutation declares is reported as broken", @@ -275,12 +312,12 @@ def test_a_source_that_does_not_import_though_its_error_is_declared(self): ) def test_a_test_that_raises_the_declared_error(self): """Test that a mutation whose test raises the declared error is reported as killed.""" - mutation = Mutation(mutation_subject, _EVEN, _ERRORING, raises=_NAME_ERROR) + mutation = Mutation(mutation_subject.is_even, _EVEN, _ERRORING, raises=_NAME_ERROR) self.assertEqual(mutation.check(_SUBJECT_TEST_NAME), Result(Outcome.KILLED)) @kills( Mutation( - checker, + checker.Mutation.check, "Result(Outcome.BROKEN, raised)", "Result(Outcome.BROKEN)", "a mutation reported as broken does not name the error the test raised, which is the line to declare", @@ -290,13 +327,13 @@ def test_a_test_that_raises_an_undeclared_error(self): """Test that a mutation whose test raises an undeclared error is reported as broken, naming the error.""" declared = "TypeError: an error the test does not raise" self.assertEqual( - Mutation(mutation_subject, _EVEN, _ERRORING, raises=declared).check(_SUBJECT_TEST_NAME), + Mutation(mutation_subject.is_even, _EVEN, _ERRORING, raises=declared).check(_SUBJECT_TEST_NAME), Result(Outcome.BROKEN, _NAME_ERROR), ) @kills( Mutation( - checker, + checker.Mutation.check, "except _SourceError as error:", "except Exception as error:", "an error in the checker itself is caught and misreported as a broken mutation", @@ -305,7 +342,7 @@ def test_a_test_that_raises_an_undeclared_error(self): def test_a_defect_in_the_checker_itself(self): """Test that an error from the checker is raised, rather than reported as the mutation being broken.""" with patch.object(Mutation, "_purge", Mock(side_effect=RuntimeError("the checker is broken"))): - self.assertRaises(RuntimeError, Mutation(mutation_subject, _EVEN, _ODD).check, _SUBJECT_TEST_NAME) + self.assertRaises(RuntimeError, Mutation(mutation_subject.is_even, _EVEN, _ODD).check, _SUBJECT_TEST_NAME) def test_the_modules_are_left_as_they_were(self): """Test that a check restores sys.modules, whether the mutated source ran or raised instead. @@ -316,7 +353,7 @@ def test_the_modules_are_left_as_they_were(self): for case, new, outcome in (("ran", _ODD, Outcome.KILLED), ("raised", "number %", Outcome.BROKEN)): with self.subTest(case=case): imported = dict(sys.modules) - result = Mutation(mutation_subject, _EVEN, new).check(_SUBJECT_TEST_NAME) + result = Mutation(mutation_subject.is_even, _EVEN, new).check(_SUBJECT_TEST_NAME) self.assertEqual(result.outcome, outcome) names = sys.modules.keys() | imported.keys() changed = [name for name in names if sys.modules.get(name) is not imported.get(name)] @@ -334,13 +371,13 @@ def test_a_mutation_whose_file_cannot_be_read(self): """Test that a mutation naming a file that cannot be read is reported as stale, and says so.""" unreadable = Mock(side_effect=FileNotFoundError(2, "No such file or directory")) with patch("pathlib.Path.read_text", unreadable): - result = Mutation(mutation_subject, _EVEN, _ODD).check(_SUBJECT_TEST_NAME) + result = Mutation(mutation_subject.is_even, _EVEN, _ODD).check(_SUBJECT_TEST_NAME) self.assertEqual(result.outcome, Outcome.STALE) self.assertIn("FileNotFoundError: [Errno 2] No such file or directory", result.reason) @kills( Mutation( - checker._span, + checker._definitions, "raise StaleError(_reason(error)) from error", "raise error", "the run ends with a traceback where a source no longer parses, rather than reporting it stale", @@ -348,12 +385,39 @@ def test_a_mutation_whose_file_cannot_be_read(self): ) ) def test_a_mutation_whose_source_does_not_parse(self): - """Test that a mutation whose file no longer parses is reported as stale, and says so.""" - unparsable = Mock(return_value="def broken(") - with patch("pathlib.Path.read_text", unparsable): - result = Mutation(Doubler.doubled, _DOUBLING, _TRIPLING).check(_DOUBLER_TEST_NAME) - self.assertEqual(result.outcome, Outcome.STALE) - self.assertIn("SyntaxError", result.reason) + """Test that a mutation whose file no longer parses is reported as stale, and says so, whatever its anchor.""" + for case, mutation in ( + ("function anchor", Mutation(Doubler.doubled, _DOUBLING, _TRIPLING)), + ("module anchor", Mutation(mutation_subject, "def broken(", "def mended(")), + ): + with self.subTest(case=case), patch("pathlib.Path.read_text", Mock(return_value="def broken(")): + result = mutation.check(_DOUBLER_TEST_NAME) + self.assertEqual(result.outcome, Outcome.STALE) + self.assertIn("SyntaxError", result.reason) + + def test_a_module_anchor_on_the_last_function_of_a_file_without_a_final_newline(self): + """Test that a module anchor on the last function's snippet is stale, though the file lacks a final newline.""" + with patch("pathlib.Path.read_text", Mock(return_value="def last():\n return 1")): + result = Mutation(mutation_subject, "return 1", "return 2").check(_SUBJECT_TEST_NAME) + self.assertEqual(result, Result(Outcome.STALE, "the snippet lies inside last, so anchor the mutation on last")) + + @kills( + Mutation( + checker._definitions, + " definitions.setdefault(name, node)", + " definitions[name] = node", + "a setter hides its getter, so a module anchor is accepted around a snippet the getter holds", + ) + ) + def test_a_module_anchor_on_a_getter_whose_property_has_a_setter(self): + """Test that a module anchor on a getter's snippet is stale, though the setter shares the getter's name.""" + source = ( + "class A:\n @property\n def x(self):\n return self._x + 1\n\n" + " @x.setter\n def x(self, value):\n self._x = value\n" + ) + with patch("pathlib.Path.read_text", Mock(return_value=source)): + result = Mutation(mutation_subject, "self._x + 1", "self._x + 2").check(_DOUBLER_TEST_NAME) + self.assertEqual(result, Result(Outcome.STALE, "the snippet lies inside A.x, so anchor the mutation on A.x")) @kills( Mutation( @@ -365,10 +429,83 @@ def test_a_mutation_whose_source_does_not_parse(self): ) def test_a_mutation_that_would_change_nothing(self): """Test that a replacement equal to the snippet is reported as stale, rather than as the test's failing.""" - result = Mutation(mutation_subject, _EVEN, _EVEN).check(_SUBJECT_TEST_NAME) + result = Mutation(mutation_subject.is_even, _EVEN, _EVEN).check(_SUBJECT_TEST_NAME) self.assertEqual(result.outcome, Outcome.STALE) self.assertEqual(result.reason, "the snippet and its replacement are the same, so nothing changes") + @kills( + Mutation( + checker._function_holding, + "first <= start and end <= last", + "False", + "a module anchor is accepted around a snippet one function holds, so the anchor claims the whole file", + ), + _CLASSES_SKIPPED, + Mutation( + checker._named, + 'f"{prefix}{node.name}."', + "prefix", + "the message names a method without its class, and an anchor cannot reach the method by that name", + ), + Mutation( + checker._named, + ' yield f"{prefix}{node.name}", node', + ' yield from _named(node.body, f"{prefix}{node.name}.")\n' + ' yield f"{prefix}{node.name}", node', + "the message names a nested function instead of the function around it, and an anchor cannot reach it", + ), + _SPAN_STOPS_BEFORE_THE_NEWLINE, + ) + def test_a_module_anchor_on_a_snippet_one_function_holds(self): + """Test that a module anchor on a snippet one function or method holds is reported as stale, naming it.""" + for case, old, new, test_name, function in ( + ("function", _EVEN, _ODD, _SUBJECT_TEST_NAME, "is_even"), + ("method", "cls().doubled(2)", "cls().doubled(3)", _TWO_DOUBLED_TEST_NAME, "Doubler.two_doubled"), + ("nested function", "count > 0", "count > 1", _SUBJECT_TEST_NAME, "is_positive_even"), + ("ending in the newline after the last line", f"{_EVEN}\n", f"{_ODD}\n", _SUBJECT_TEST_NAME, "is_even"), + ): + with self.subTest(case=case): + result = Mutation(mutation_subject, old, new).check(test_name) + self.assertEqual(result.outcome, Outcome.STALE) + self.assertEqual( + result.reason, f"the snippet lies inside {function}, so anchor the mutation on {function}" + ) + + @kills( + Mutation( + checker._function_holding, + "first <= start and end <= last", + "True", + "a module anchor is rejected around a snippet outside every function, though a function cannot anchor it", + ), + Mutation( + checker._node_span, + "_offset(source, node.lineno, node.col_offset)", + "_offset(source, min([node.lineno, " + '*(decorator.lineno for decorator in getattr(node, "decorator_list", []))]), 0)', + "a span starts at the decorators, so a snippet starting at one is rejected, though its function lacks it", + ), + Mutation( + checker._function_holding, + "first <= start and end <= last", + "first <= start <= last", + "a snippet starting in one function and ending in the next is rejected as if the first held it all", + ), + ) + def test_a_module_anchor_on_a_snippet_no_function_holds_on_its_own(self): + """Test that a module anchor on a snippet no function or method holds on its own is applied.""" + for case, old, new in ( + ( + "decorator", + " @property\n def one_doubled(self)", + " @property # applied\n def one_doubled(self)", + ), + ("two functions", "== 0\n\n\ndef is_positive_even", "== 0\n\n\n\ndef is_positive_even"), + ): + with self.subTest(case=case): + result = Mutation(mutation_subject, old, new).check(_SUBJECT_TEST_NAME) + self.assertEqual(result, Result(Outcome.SURVIVED)) + @kills( Mutation( checker.mutated_source, @@ -511,13 +648,13 @@ class FailureMessageTest(unittest.TestCase): @kills( Mutation( - checker, + checker.Mutation._anchor_name, '".".join(filter(None, (self._module.__name__, self._qualified_name)))', "self._module.__name__", "a report names the module alone, so it does not say which of its anchors went stale", ), Mutation( - checker, + checker.Mutation._anchor_name, "filter(None, (self._module.__name__, self._qualified_name))", "(self._module.__name__, self._qualified_name)", "a module anchor is reported with a trailing dot, as though a definition were missing from the name", @@ -565,13 +702,13 @@ def run_decorated_test( @kills( Mutation( - checker, + checker._fail_unless_killed, " for mutation in mutations:", " for mutation in mutations[:1]:", "only the first of the mutations a registration holds is checked", ), Mutation( - checker, + checker._fail_unless_killed, " result = mutation.check(test_name)", ' result = mutation.check(test_name.split(".")[-1])', "the test is named without the module it sits in, so the checker cannot load it", @@ -608,19 +745,19 @@ def test_a_surviving_mutation_fails_the_test(self): @kills( Mutation( - checker, + checker._fail_unless_killed, " with test_case.subTest(regression=mutation.regression):", " if True:", "a test stops at the first mutation it did not kill, leaving every mutation after it unreported", ), Mutation( - checker, + checker._fail_unless_killed, " with test_case.subTest(regression=mutation.regression):", " with test_case.subTest():", "a mutation the test did not kill is reported without naming which mutation it was", ), Mutation( - checker, + checker._fail_unless_killed, "_failure(mutation, result))", "_failure(mutations[0], result))", "every mutation the test did not kill is reported with the first one's regression", @@ -655,7 +792,8 @@ def test_a_mutation_the_test_could_not_judge_fails_it_with_the_reason(self): self.assertEqual(len(result.failures), 1) message = result.failures[0][1] self.assertIn( - f"{_REGRESSION} — this mutation of {mutation_subject.__name__} is {outcome}: {reason}", message + f"{_REGRESSION} — this mutation of {is_even.__module__}.{is_even.__name__} is {outcome}: {reason}", + message, ) def test_a_test_failing_of_its_own_accord_is_not_checked(self): @@ -667,13 +805,13 @@ def test_a_test_failing_of_its_own_accord_is_not_checked(self): @kills( Mutation( - checker, + checker.kills, ' if os.environ.get(CHECKED_TEST) == self.id() or os.environ.get(CHECKS_OFF) == "1":', " if os.environ.get(CHECKS_OFF):", "a test re-run against its own mutation checks its mutations again, so a run never ends", ), Mutation( - checker, + checker.kills, ' if os.environ.get(CHECKED_TEST) == self.id() or os.environ.get(CHECKS_OFF) == "1":', " if os.environ.get(CHECKED_TEST) == self.id():", "a `just mutate` run checks the registered mutations too, so its kill list names tests it never broke", @@ -709,7 +847,7 @@ def test_the_sentinel_survives_a_cleared_environment(self): @kills( Mutation( - checker, + checker.kills, ' if os.environ.get(CHECKED_TEST) == self.id() or os.environ.get(CHECKS_OFF) == "1":', " if os.environ.get(CHECKED_TEST) or os.environ.get(CHECKS_OFF):", "a decorated test the checked test reaches stands aside too, so its mutations go unchecked", @@ -723,7 +861,7 @@ def test_a_test_the_sentinel_does_not_name_checks_its_mutations(self): @kills( Mutation( - checker, + checker.kills, " setattr(wrapper, _REGISTERED, mutations)", " pass", "a decorated test carries no registration, so a second one on it goes undetected", @@ -747,7 +885,7 @@ def test_a_second_registration_on_one_test_fails_it(self): @kills( Mutation( - checker, + checker.kills, " if not mutations:", " if False:", "a registration naming no mutation is accepted, so the test it decorates checks nothing", @@ -762,7 +900,7 @@ def test_a_registration_of_no_mutation_fails_the_test(self): @kills( Mutation( - checker, + checker.kills, " @functools.wraps(method)", "", "a decorated test reports under the wrapper's name, so a failing run names no test", diff --git a/tests/tools/test_callers.py b/tests/tools/test_callers.py index e832217..2b3ce13 100644 --- a/tests/tools/test_callers.py +++ b/tests/tools/test_callers.py @@ -22,7 +22,7 @@ def sites(self, name: str, *texts: str) -> list[str]: @kills( Mutation( - callers_module, + callers_module._calls_name, ' return ast.unparse(call.func).rsplit(".", 1)[-1] == name', " return ast.unparse(call.func) == name", "a call made on an object is missed, so the count comes out short", @@ -41,7 +41,7 @@ def test_every_kind_of_call(self): @kills( Mutation( - callers_module, + callers_module.call_sites, ' return [f"{path}:{call.lineno}: {ast.unparse(call)}"', ' return [f"{path}:{call.end_lineno}: {ast.unparse(call)}"', "a call is reported at the line its arguments end on, so the report points past the call it names", @@ -53,7 +53,7 @@ def test_a_call_spanning_several_lines(self): @kills( Mutation( - callers_module, + callers_module._calls, " return [node for node in ast.walk(_parsed(path)) " "if isinstance(node, ast.Call) and _calls_name(node, name)]", ' return [node for node in ast.walk(_parsed(path)) if getattr(node, "name", None) == name ' @@ -61,7 +61,7 @@ def test_a_call_spanning_several_lines(self): "the line defining the name counts as a call site, which is the miscount a grep makes", ), Mutation( - callers_module, + callers_module._calls_name, ' return ast.unparse(call.func).rsplit(".", 1)[-1] == name', ' return name in ast.unparse(call.func).split(".")', "a call made on the name counts as a call to it, so a method call inflates the count", @@ -112,7 +112,7 @@ def test_the_number_of_call_sites(self): @kills( Mutation( - callers_module, + callers_module.main, " undefined = not sites and not _defined(name, files)", " undefined = not sites", "a name nothing calls fails the run, so a name nobody uses cannot be told from a misspelled one", diff --git a/tests/tools/test_mutate.py b/tests/tools/test_mutate.py index 41fe3cd..61004e7 100644 --- a/tests/tools/test_mutate.py +++ b/tests/tools/test_mutate.py @@ -58,13 +58,18 @@ def probe( return main() def test_the_file_is_mutated_and_restored(self): - """Test that the snippet is replaced, the command run, and the file put back as it was.""" - path = mock_path(_ORIGINAL) - self.probe(path) - self.assertEqual(path.write_text.call_args_list, [call("before\nnew\nafter\n"), call(_ORIGINAL)]) - self.run_command.assert_called_once_with( - ["just", "test"], check=False, capture_output=True, text=True, env=_ENVIRONMENT - ) + """Test that the snippet is replaced, inside a function too, the command run, and the file put back.""" + for case, original, mutated in ( + ("at module level", _ORIGINAL, "before\nnew\nafter\n"), + ("inside a function", "def one():\n old\n", "def one():\n new\n"), + ): + with self.subTest(case=case): + path = mock_path(original) + self.probe(path) + self.assertEqual(path.write_text.call_args_list, [call(mutated), call(original)]) + self.run_command.assert_called_once_with( + ["just", "test"], check=False, capture_output=True, text=True, env=_ENVIRONMENT + ) def test_the_kills_checks_are_skipped(self): """Test that the command runs with the mutation checks off, so its kill list names the tests that failed.""" diff --git a/tests/tools/test_readability_check.py b/tests/tools/test_readability_check.py index 5e0ffe6..14dc4b9 100644 --- a/tests/tools/test_readability_check.py +++ b/tests/tools/test_readability_check.py @@ -3,7 +3,7 @@ import io import sys import unittest -from contextlib import redirect_stdout +from contextlib import redirect_stderr, redirect_stdout from pathlib import Path from unittest.mock import Mock, patch @@ -221,19 +221,19 @@ def test_each_threshold_is_reported_on_its_own(self): @kills( Mutation( - readability_check, + readability_check._subject_is_split, " return determiners >= _SPLIT_SUBJECT_DETERMINERS\n", " return False\n", "no sentence is reported for splitting its subject from its verb, so the fault goes unreported", ), Mutation( - readability_check, + readability_check._faults, " tagged = _tagged(_without_asides(sentence))\n", " tagged = _tagged(sentence)\n", "an aside's noun phrase reads as a clause wedged into the subject, so plain prose is reported", ), Mutation( - readability_check, + readability_check._subject_is_split, " if tag == _DETERMINER and previous not in _BESIDE_THE_SUBJECT:\n", " if tag == _DETERMINER:\n", "a noun phrase standing beside the subject reads as a clause wedged into it, so plain prose is reported", @@ -265,19 +265,19 @@ def test_a_subject_split_from_its_verb_is_reported(self): @kills( Mutation( - readability_check, + readability_check._faults, " if _negates_a_noun_phrase(tagged):\n", " if False:\n", "no sentence is reported for hanging its negation on a noun phrase, so the fault goes unreported", ), Mutation( - readability_check, + readability_check._negates_a_noun_phrase, " elif seen_verb and tag == _DETERMINER", " elif tag == _DETERMINER", "a subject opening with the negation is reported, though it carries the negation to the verb", ), Mutation( - readability_check, + readability_check._negates_a_noun_phrase, "and not _denies_existence(tagged, index)", "", "`there is no new version` is reported, though that is how plain prose says it", @@ -286,7 +286,10 @@ def test_a_subject_split_from_its_verb_is_reported(self): def test_a_negation_in_a_noun_phrase_is_reported(self): """Test that a negation the sentence hangs on a noun phrase is reported, and one on its verb is not.""" cases = { - "a negation hung on an object": (self.NEGATED_NOUN, "negation in a noun phrase"), + "a negation hung on an object": ( + self.NEGATED_NOUN, + "negation in a noun phrase (put the negation on the verb, as in 'does not have a release')", + ), "a negation on the verb": (self.NEGATED_VERB, ""), "a negation opening the subject": (self.NEGATED_SUBJECT, ""), "a negation denying that anything exists": (self.NEGATED_EXISTENCE, ""), @@ -373,20 +376,14 @@ def test_missing_data_is_downloaded(self, punkt_tokenizer: Mock, nltk: Mock): class WhitelistedSentencesTest(unittest.TestCase): """Unit tests for reading the sentences the check passes over.""" - def whitelisted(self, whitelist: Mock) -> set[str]: - """Return the sentences read from the given whitelist file.""" - with patch("tools.readability_check._WHITELIST", whitelist): - return _whitelisted_sentences() - def test_a_sentence_per_line(self): """Test that the file holds one sentence per line, and that every line is read as one.""" - self.assertEqual( - self.whitelisted(mock_path("One sentence.\nAnother sentence.\n")), {"One sentence.", "Another sentence."} - ) + whitelist = mock_path("One sentence.\nAnother sentence.\n") + self.assertEqual(_whitelisted_sentences(whitelist), {"One sentence.", "Another sentence."}) def test_no_whitelist_file(self): - """Test that a tree without a whitelist file passes over nothing.""" - self.assertEqual(self.whitelisted(Mock(exists=Mock(return_value=False))), set()) + """Test that a whitelist file that does not exist passes over nothing.""" + self.assertEqual(_whitelisted_sentences(Mock(exists=Mock(return_value=False))), set()) @patch("tools.readability_check.extract_prose") @@ -406,29 +403,38 @@ def check( extract: Mock, *texts: str, arguments: tuple[str, ...] = ("src",), - whitelist: tuple[str, ...] = (), + whitelist: tuple[str, ...] | None = (), + option: str = "--whitelist", ) -> tuple[int, str]: - """Run the check over the given runs of prose, and return its exit code and what it wrote.""" + """Run the check over the runs of prose, and return its exit code and output.""" extract.return_value = [Prose(Path("conf.py"), text, 1) for text in texts] tokenizer.return_value.span_tokenize.side_effect = lambda masked: [(0, len(masked))] written = io.StringIO() - held = patch(f"{main.__module__}._whitelisted_sentences", Mock(return_value=set(whitelist))) - with redirect_stdout(written), patch.object(sys, "argv", ["check", *arguments]), held: + files = {} if whitelist is None else {Path("prose.txt"): "".join(f"{sentence}\n" for sentence in whitelist)} + read = patch.object(Path, "read_text", autospec=True, side_effect=lambda path: files[path]) + found = patch.object(Path, "exists", autospec=True, side_effect=lambda path: path in files) + argv = ["check", *(() if whitelist is None else (option, "prose.txt")), *arguments] + with redirect_stdout(written), patch.object(sys, "argv", argv), read, found: return main(), written.getvalue() def test_a_stale_whitelist_entry_is_reported(self, tokenizer: Mock, extract: Mock): - """Test that a whitelist entry the run does not match is reported, and fails the run.""" + """Test that an entry of the file `--check-whitelist` names that the run does not match is reported.""" stale = r"A sentence the prose no longer holds." - arguments = ("--check-whitelist", "src") whitelist = (self.NESTED, stale) # The flagged sentence is held, so the stale entry is the only report. - exit_code, written = self.check(tokenizer, extract, self.NESTED, arguments=arguments, whitelist=whitelist) + exit_code, written = self.check( + tokenizer, extract, self.NESTED, whitelist=whitelist, option="--check-whitelist" + ) self.assertEqual(exit_code, 1) - self.assertIn(stale, written) + self.assertIn( + f"prose.txt holds a sentence the prose no longer has, run `just update-whitelists`:\n{stale}", written + ) def test_a_whitelist_entry_that_is_still_needed(self, tokenizer: Mock, extract: Mock): """Test that an entry matching a sentence the run flags is left unreported, and leaves the run passing.""" - arguments = ("--check-whitelist", "src") - exit_code, written = self.check(tokenizer, extract, self.NESTED, arguments=arguments, whitelist=(self.NESTED,)) + whitelist = (self.NESTED,) + exit_code, written = self.check( + tokenizer, extract, self.NESTED, whitelist=whitelist, option="--check-whitelist" + ) self.assertEqual(exit_code, 0) self.assertEqual(written, "") @@ -438,6 +444,33 @@ def test_hard_to_read_sentence(self, tokenizer: Mock, extract: Mock): self.assertEqual(exit_code, 1) self.assertIn("conf.py:1: complexity 6:", written) + def test_a_run_naming_no_whitelist(self, tokenizer: Mock, extract: Mock): + """Test that a run without `--whitelist` reports every flagged sentence, without reading a file.""" + exit_code, written = self.check(tokenizer, extract, self.NESTED, whitelist=None) + self.assertEqual(exit_code, 1) + self.assertIn("conf.py:1: complexity 6:", written) + + def test_naming_the_whitelist_twice_is_an_error(self, _tokenizer: Mock, _extract: Mock): + """Test that a run naming the whitelist in two options at once ends with a usage error.""" + cases = { + "plain and check": ("--whitelist", "--check-whitelist"), + "check and make": ("--check-whitelist", "--make-whitelist"), + } + for case, (first, second) in cases.items(): + with self.subTest(case=case): + argv = ["check", first, "one.txt", second, "two.txt", "src"] + with ( + patch.object(sys, "argv", argv), + patch.object(Path, "exists", autospec=True, return_value=False), + patch.object(Path, "write_text", autospec=True) as write_text, + redirect_stdout(io.StringIO()), + redirect_stderr(io.StringIO()), + self.assertRaises(SystemExit) as raised, + ): + main() + self.assertEqual(raised.exception.code, 2) + write_text.assert_not_called() + def test_readable_sentence(self, tokenizer: Mock, extract: Mock): """Test that a sentence under every threshold is not written out, and leaves the run passing.""" exit_code, written = self.check(tokenizer, extract, "A plain sentence.") @@ -456,20 +489,27 @@ def test_a_whitelisted_sentence_the_prose_wraps(self, tokenizer: Mock, extract: self.assertEqual(exit_code, 0) self.assertEqual(written, "") + def make_whitelist( + self, tokenizer: Mock, extract: Mock, *texts: str, whitelist: tuple[str, ...] + ) -> tuple[Path, str]: + """Make the whitelist from the given runs of prose, and return the file the run wrote and its text.""" + with patch.object(Path, "write_text", autospec=True) as write_text: + exit_code, written = self.check(tokenizer, extract, *texts, whitelist=whitelist, option="--make-whitelist") + self.assertEqual((exit_code, written), (0, "")) + path, text = write_text.call_args.args + return path, text + def test_making_the_whitelist(self, tokenizer: Mock, extract: Mock): - """Test that the flagged sentence is written out on one line to be whitelisted, rather than reported.""" - arguments = ("--make-whitelist", "src") - exit_code, written = self.check(tokenizer, extract, self.WRAPPED, arguments=arguments) - self.assertEqual(exit_code, 0) - self.assertEqual(written, f"{self.NESTED}\n") + """Test that the file `--make-whitelist` names keeps a flagged sentence on one line only where it holds it.""" + path, text = self.make_whitelist(tokenizer, extract, self.WRAPPED, self.ASIDE, whitelist=(self.NESTED,)) + self.assertEqual(path, Path("prose.txt")) + self.assertEqual(text, f"{self.NESTED}\n") def test_the_whitelist_holds_each_sentence_once(self, tokenizer: Mock, extract: Mock): - """Test that a sentence two files hold is written out once, and that the sentences come out sorted. + """Test that a sentence two files hold is written once, and that the sentences come out sorted. Regenerating the whitelist then rewrites the lines that changed rather than reshuffling the whole file. """ - arguments = ("--make-whitelist", "src") texts = (self.ASIDE, self.NESTED, self.ASIDE) - exit_code, written = self.check(tokenizer, extract, *texts, arguments=arguments) - self.assertEqual(exit_code, 0) - self.assertEqual(written, f"{self.NESTED}\n{self.ASIDE}\n") + _path, text = self.make_whitelist(tokenizer, extract, *texts, whitelist=(self.ASIDE, self.NESTED)) + self.assertEqual(text, f"{self.NESTED}\n{self.ASIDE}\n") diff --git a/tests/tools/test_readme_structure_check.py b/tests/tools/test_readme_structure_check.py index 26654b3..e2697df 100644 --- a/tests/tools/test_readme_structure_check.py +++ b/tests/tools/test_readme_structure_check.py @@ -97,7 +97,7 @@ def test_document_without_type_sections_is_reported(self): @kills( Mutation( - structure_check, + structure_check._problems, " + _type_section_problems(markdown)\n", "", "a declared dependency type the details chapter has no section for goes unreported", @@ -111,7 +111,7 @@ def test_declared_type_without_a_section_is_reported(self): @kills( Mutation( - structure_check, + structure_check._type_section_problems, """ problems += [ f"the details chapter's section '{title}' documents no dependency type" for title in titles @@ -130,7 +130,7 @@ def test_section_documenting_no_declared_type_is_reported(self): @kills( Mutation( - structure_check, + structure_check._problems, " + _files_problems(markdown)\n", "", "a file a dependency type declares that the README's Files table leaves out goes unreported", @@ -196,7 +196,7 @@ def test_alignment_colons_in_the_separator_are_not_a_row(self): @kills( Mutation( - structure_check, + structure_check._table_problems, 'for problem in _list_problems(_row_labels(rows), list(_TYPE_NAMES), f"the \'{header}\' table", "row")', 'for problem in _list_problems(_row_labels(rows), _row_labels(rows), f"the \'{header}\' table", "row")', "each table is compared with itself, so no table is held to the declared dependency types", diff --git a/tests/tools/test_rename.py b/tests/tools/test_rename.py index b5851b6..2652635 100644 --- a/tests/tools/test_rename.py +++ b/tests/tools/test_rename.py @@ -133,7 +133,7 @@ def test_the_files_the_rename_changed_are_written(self): @kills( Mutation( - rename_module, + rename_module.main, " return _FAILED\n for path, source in changed.items():", " for path, source in changed.items():\n Path(path).write_text(source)\n" " return _FAILED\n for path, source in changed.items():", @@ -147,7 +147,7 @@ def test_a_rename_that_left_the_name_behind_writes_nothing(self): @kills( Mutation( - rename_module, + rename_module._renamed_sources, ' _report(f"{path} could not be renamed: {reason}")', ' _report(f"{path} could not be renamed: {reason}")\n' " for written, renamed_source in renamed.items():\n" @@ -173,7 +173,7 @@ def test_a_rename_that_changed_nothing(self): @kills( Mutation( - rename_module, + rename_module._survivors_message, "', '.join(left)", "' '.join(left)", "the surviving occurrences are run together without the comma between them", diff --git a/tests/update_time/domain/test_changelog.py b/tests/update_time/domain/test_changelog.py index d443f3c..8bc2ea9 100644 --- a/tests/update_time/domain/test_changelog.py +++ b/tests/update_time/domain/test_changelog.py @@ -24,7 +24,7 @@ class MarkupTest(unittest.TestCase): @kills( Mutation( - changelog, + changelog.is_markdown_file, " return urlparse(name).path.lower().endswith(MARKDOWN_EXTENSION)", " return urlparse(name).path.endswith(MARKDOWN_EXTENSION)", "a changelog file that shouts its extension is read as text, so its Markdown is shown raw", @@ -36,13 +36,13 @@ def test_an_extension_is_read_whatever_its_case(self): @kills( Mutation( - changelog, + changelog.is_markdown_content_type, ' return content_type.partition(";")[0].strip().lower() == _MARKDOWN_CONTENT_TYPE', ' return content_type.partition(";")[0].lower() == _MARKDOWN_CONTENT_TYPE', "a content type padded with spaces matches nothing, so its Markdown is shown raw", ), Mutation( - changelog, + changelog.is_markdown_content_type, ' return content_type.partition(";")[0].strip().lower() == _MARKDOWN_CONTENT_TYPE', ' return content_type.partition(";")[0].strip() == _MARKDOWN_CONTENT_TYPE', "a content type spelled in another case matches nothing, so its Markdown is shown raw", @@ -83,14 +83,14 @@ def test_prose_mention_of_version_does_not_anchor_parsing(self): self.assertEqual(get_version_changes_from_changelog(text, "1.0.0"), v1_change) _HASH_ANCHOR = Mutation( - changelog, + changelog._find_version_index, " if _heading_level(lines, index):", ' if line.startswith("#"):', "a version named in a newer entry's prose anchors the changes reported for it", ) _NO_UNDERLINE = Mutation( - changelog, + changelog._heading_level, " return _underline_character(lines, index)", ' return ""', "a reStructuredText changelog reports the changes from where a newer entry's prose names the version", @@ -105,7 +105,7 @@ def test_prose_mention_of_version_does_not_anchor_parsing(self): ) _FENCE_OVER_UNDERLINE = Mutation( - changelog, + changelog._underlines_the_line_above, " return index > 0 and _heading_level(lines, index - 1) == lines[index][:1]", " return False", "a changelog underlining its headings with backticks or tildes reports no changes at all, its file read " @@ -129,14 +129,14 @@ def test_repeated_prose_mention_without_heading_anchors_on_first(self): self.assertEqual(get_version_changes_from_changelog(text, "1.0.0"), text) _NO_LOOKAHEAD = Mutation( - changelog, + changelog._names_version, ' return re.search(rf"{_VERSION_START}{re.escape(version)}(?!\\.?\\w)", line) is not None', ' return re.search(rf"{_VERSION_START}{re.escape(version)}", line) is not None', "a changelog naming a longer version that starts with this one reports the longer version's changes", ) _NO_LOOKBEHIND = Mutation( - changelog, + changelog._names_version, ' return re.search(rf"{_VERSION_START}{re.escape(version)}(?!\\.?\\w)", line) is not None', ' return re.search(rf"{re.escape(version)}(?!\\.?\\w)", line) is not None', "a changelog naming a longer version that ends with this one reports the longer version's changes", @@ -153,7 +153,7 @@ def test_longer_version_does_not_anchor_parsing(self): self.assertEqual(get_version_changes_from_changelog(text, version), v1_change) _NO_SHORTER_HEADING = Mutation( - changelog, + changelog.get_version_changes_from_changelog, ' if start is None and version.endswith(".0") and version.count(".") > 1:\n' ' version = version.removesuffix(".0")\n' " start = _find_version_index(all_lines, version)", @@ -162,7 +162,7 @@ def test_longer_version_does_not_anchor_parsing(self): ) _SHORTEN_ANY_VERSION = Mutation( - changelog, + changelog.get_version_changes_from_changelog, ' if start is None and version.endswith(".0") and version.count(".") > 1:\n' ' version = version.removesuffix(".0")', ' if start is None and version.count(".") > 1:\n version = version.rsplit(".", 1)[0]', @@ -178,7 +178,7 @@ def test_only_a_dot_zero_version_is_found_under_a_shorter_heading(self): self.assertEqual(get_version_changes_from_changelog(text, "1.11.2"), "") _SHORTER_VERSION_NOT_REBOUND = Mutation( - changelog, + changelog.get_version_changes_from_changelog, ' version = version.removesuffix(".0")\n start = _find_version_index(all_lines, version)', ' start = _find_version_index(all_lines, version.removesuffix(".0"))', "a changelog without heading markup naming a shorter version raises instead of reporting its changes", @@ -193,7 +193,7 @@ def test_dot_zero_version_is_found_under_a_shorter_version_named_in_prose(self): self.assertEqual(get_version_changes_from_changelog(text, "1.11.0"), v1_change) _SHORTER_HEADING_TRIED_FIRST = Mutation( - changelog, + changelog.get_version_changes_from_changelog, " if start is None and", ' if _find_version_index(all_lines, version.removesuffix(".0")) is not None and', "a changelog heading both a version and the release one component shorter reports the shorter one's changes", @@ -207,7 +207,7 @@ def test_heading_naming_the_version_wins_over_a_shorter_heading(self): self.assertEqual(get_version_changes_from_changelog(text, "1.11.0"), v1_change) _SHORTEN_TO_A_BARE_COMPONENT = Mutation( - changelog, + changelog.get_version_changes_from_changelog, ' if start is None and version.endswith(".0") and version.count(".") > 1:', ' if start is None and version.endswith(".0"):', "a changelog naming a two-component version nowhere reports the entry a stray digit sits in", @@ -221,7 +221,7 @@ def test_two_component_version_is_not_found_under_a_bare_component(self): self.assertEqual(get_version_changes_from_changelog(text, "1.0"), "") _FENCED_ANCHOR = Mutation( - changelog, + changelog._find_version_index, " if index not in fenced and _names_version(line, version):", " if _names_version(line, version):", "a version heading inside a fenced code block anchors the changes reported for that version", @@ -262,7 +262,7 @@ def test_next_version_heading_ends_the_section_at_any_level(self): self.assertEqual(get_version_changes_from_changelog(text, "1.0"), v1_change) _DEEPER_LEVEL = Mutation( - changelog, + changelog._ends_section, " return heading_level == section_level or (", " return heading_level.startswith(section_level) or (", "a Markdown entry ends at its first subsection, reporting the version's heading alone", @@ -276,7 +276,7 @@ def test_deeper_markdown_heading_does_not_end_the_section(self): self.assertEqual(get_version_changes_from_changelog(text, "1.0"), v1_change) _NO_LONGER_VERSION_END = Mutation( - changelog, + changelog._next_version_index, " or _heads_a_longer_version(heading_level, line, version)", "", "a changelog heading a longer version inside a version's section reports that longer version's entry as well", @@ -307,7 +307,7 @@ def test_heading_inside_a_fenced_code_block_does_not_end_the_section(self): self.assertEqual(get_version_changes_from_changelog(text, "1.0"), v1_change) _EQUAL_LEVEL_ONLY = Mutation( - changelog, + changelog._ends_section, ' return heading_level == section_level or (heading_level.startswith("#") ' "and len(heading_level) < len(section_level))", " return heading_level == section_level", @@ -328,7 +328,7 @@ def test_underlined_heading_of_next_version_ends_the_section(self): self.assertEqual(get_version_changes_from_changelog(text, "1.0"), v1_change) _ANY_LEVEL = Mutation( - changelog, + changelog._ends_section, " return heading_level == section_level or (", " return bool(heading_level) or (", "a reStructuredText section ends at its first subsection, reporting the version's heading alone", @@ -342,7 +342,7 @@ def test_subsection_heading_does_not_end_the_section(self): self.assertEqual(get_version_changes_from_changelog(text, "1.0"), v1_change) _NO_BLANK_GUARD = Mutation( - changelog, + changelog._heading_level, " if not line.strip():", " if False:", "a run of punctuation that underlines nothing cuts the entry short where it stands", @@ -356,7 +356,7 @@ def test_punctuation_run_underlining_nothing_does_not_end_the_section(self): self.assertEqual(get_version_changes_from_changelog(text, "1.0"), v1_change) _STRIPPED_UNDERLINE = Mutation( - changelog, + changelog._underline_character, ' below = lines[index + 1].rstrip() if index + 1 < len(lines) else ""', ' below = lines[index + 1].strip() if index + 1 < len(lines) else ""', "a run of punctuation indented inside a literal block cuts the entry short above it", @@ -370,14 +370,14 @@ def test_indented_punctuation_run_does_not_end_the_section(self): self.assertEqual(get_version_changes_from_changelog(text, "1.0"), v1_change) _HEADING_PREFIX = Mutation( - changelog, + changelog._version_line_prefix, " return None if _heading_level(lines, index) else line[: line.index(version)]", " return line[: line.index(version)]", "a heading-anchored entry ends where it names another version in prose", ) _PROSE_NAMING_A_LONGER_VERSION = Mutation( - changelog, + changelog._heads_a_longer_version, " return bool(heading_level) and re.search", " return re.search", "an entry ends where its prose names a longer version, reporting the lines above that alone", @@ -391,7 +391,7 @@ def test_version_line_does_not_end_a_heading_anchored_section(self): self.assertEqual(get_version_changes_from_changelog(text, "1.0"), v1_change) _ANY_VERSION_LINE = Mutation( - changelog, + changelog._heads_another_version, " if not line.startswith(prefix):\n return False\n" " other_version = _VERSION.match(line, len(prefix))", " other_version = _VERSION.search(line)", @@ -406,7 +406,7 @@ def test_prose_naming_a_version_does_not_end_the_section(self): self.assertEqual(get_version_changes_from_changelog(text, "1.0"), v1_change) _NO_PREFIX = Mutation( - changelog, + changelog._version_line_prefix, " return None if _heading_level(lines, index) else line[: line.index(version)]", ' return None if _heading_level(lines, index) else ""', "a changelog naming its versions with text around them runs on into the previous version", @@ -423,7 +423,7 @@ def test_next_version_line_ends_the_section_without_heading_markup(self): self.assertEqual(get_version_changes_from_changelog(text, "1.0"), v1_change) _NO_HEADING_BOUND = Mutation( - changelog, + changelog._ends_section, " return bool(heading_level)", " return False", "a version named in prose rather than in a heading reports the rest of the changelog as its changes", @@ -457,7 +457,7 @@ def test_maximum_length_is_not_applied_when_previous_version_is_found(self): self.assertEqual(get_version_changes_from_changelog(text, "1.0"), v1_change) _NO_SAME_VERSION_SKIP = Mutation( - changelog, + changelog._next_version_index, " if index in fenced or _names_version(line, version):", " if index in fenced:", "a changelog naming a version in two headings reports the first of them's entry alone", @@ -471,7 +471,7 @@ def test_second_heading_naming_the_version_does_not_end_the_section(self): self.assertEqual(get_version_changes_from_changelog(text, "1.0"), v1_change) _BOUNDARY_CONTAINMENT = Mutation( - changelog, + changelog._next_version_index, " if index in fenced or _names_version(line, version):", " if index in fenced or version in line:", "a changelog heading a release candidate below its release reports the candidate's entry as the release's", diff --git a/tests/update_time/domain/test_dependency.py b/tests/update_time/domain/test_dependency.py index bb50d97..74d0c73 100644 --- a/tests/update_time/domain/test_dependency.py +++ b/tests/update_time/domain/test_dependency.py @@ -42,7 +42,7 @@ class ReleaseTest(unittest.TestCase): @kills( Mutation( - dependency, + dependency.Release._sortable_version, " return Version(self.version) if is_valid(self.version) else LOWEST_VERSION", " return LOWEST_VERSION", "releases published at one moment order by the order the source listed them in", diff --git a/tests/update_time/helpers.py b/tests/update_time/helpers.py index 6e8bb2d..a2b4428 100644 --- a/tests/update_time/helpers.py +++ b/tests/update_time/helpers.py @@ -789,22 +789,39 @@ def maven_central_listing(*rows: str) -> str: return f'
\n../\n{"".join(rows)}
' -def maven_central_pom(scm: str = "", tag: str = "url") -> str: +def maven_central_pom(scm: str = "", tag: str = "url", project_url: str = "", parent: str = "") -> str: """Return the pom beside a version, naming the source repository in the given child of its `` element. An scm given as nothing is a pom that does not declare an `` element at all, as guava's own pom does not. + A project URL is the project's own `` element, where guava's pom names its repository instead. A parent, + given as `groupId:artifactId:version`, is the pom's `` element. """ + url = f" {project_url}\n" if project_url else "" declared = f" \n <{tag}>{scm}\n \n" if scm else "" - return f'\n{declared}\n' + return f'\n{_pom_parent(parent)}{url}{declared}\n' -def _maven_central_api(listing: str, pom: str | None, *, archived: bool) -> Mock: +def _pom_parent(parent: str) -> str: + """Return the `` element naming the `groupId:artifactId:version` coordinates, or nothing for none. + + A coordinate given as nothing is left out of the element. + """ + if not parent: + return "" + values = dict(zip(("groupId", "artifactId", "version"), parent.split(":"), strict=True)) + coordinates = "".join(f"<{tag}>{value}" for tag, value in values.items() if value) + return f" {coordinates}\n" + + +def _maven_central_api(listing: str, pom: str | None, *, archived: bool, releases: list | None) -> Mock: """Return a requests.get mock serving an artefact's listing, a version's pom, and the GitHub repository it names. + GitHub serves the given releases for every repository, and answers the releases request non-OK where none are given. + Each request is answered by the URL it names, so a test needs no expectation about the order they are made in. A pom given as None is one the repository does not serve. """ - github = _github_api(archived=archived) + github = _github_api(releases, archived=archived) def serve(url: str, **kwargs: object) -> Mock: if urlparse(url).hostname == "api.github.com": @@ -818,9 +835,11 @@ def serve(url: str, **kwargs: object) -> Mock: return Mock(side_effect=serve) -def patch_maven_central(listing: str, pom: str | None, *, archived: bool) -> _patch: +def patch_maven_central( + listing: str, pom: str | None, *, archived: bool = False, releases: list | None = None +) -> _patch: """Patch requests.get to serve a Maven artefact's listing and pom (see `_maven_central_api`).""" - return patch("requests.get", _maven_central_api(listing, pom, archived=archived)) + return patch("requests.get", _maven_central_api(listing, pom, archived=archived, releases=releases)) def jsdelivr_versions(*version_strings: str) -> Mock: diff --git a/tests/update_time/io/test_cli.py b/tests/update_time/io/test_cli.py index 16ecc26..08f29fe 100644 --- a/tests/update_time/io/test_cli.py +++ b/tests/update_time/io/test_cli.py @@ -92,7 +92,7 @@ def test_help(self): @kills( Mutation( - cli_module, + cli_module._scanned_file_types, " file_type.name for dependency_type in DEPENDENCY_TYPES " "for file_type in dependency_type.file_types", " file_type.name for dependency_type in list(DEPENDENCY_TYPES)[:-1] " @@ -108,7 +108,7 @@ def test_help_names_the_file_types_of_every_dependency_type(self): @kills( Mutation( - cli_module, + cli_module._scanned_file_types, " dict.fromkeys(\n", " (\n", "the help names a file type twice when two dependency types declare it", @@ -124,7 +124,7 @@ def test_help_names_a_file_type_two_dependency_types_declare_once(self): @kills( Mutation( - cli_module, + cli_module._scanned_file_types, " file_types = list(\n", " file_types = sorted(\n", "the help names the file types in an order of its own rather than the one they are declared in", diff --git a/tests/update_time/io/test_console.py b/tests/update_time/io/test_console.py index 6379046..0f0e301 100644 --- a/tests/update_time/io/test_console.py +++ b/tests/update_time/io/test_console.py @@ -86,7 +86,7 @@ def assert_boxed(self, rendered: str, contents: list[str]) -> None: @kills( Mutation( - console_module, + console_module._ChangelogHandler._changes_markup, " return _ChangelogMarkdown(changes, hyperlinks=self.console.is_terminal)", " return _ChangelogMarkdown(changes, hyperlinks=True)", "a link's URL is written into an escape nothing renders, so a file or a CI log holds the text alone", @@ -102,7 +102,7 @@ def test_a_links_url_is_printed_where_nothing_can_be_clicked(self): @kills( Mutation( - console_module, + console_module._ChangelogHandler.render_message, ' if note := getattr(record, NOTE, ""):\n' " return Group(rendered, Text(note))\n" " return rendered", @@ -118,7 +118,7 @@ def test_a_record_without_changes_gets_no_empty_block(self): @kills( Mutation( - console_module, + console_module._RawHtml.__rich_console__, " html = self.html.strip()\n" ' if not (html.startswith("")):\n' ' yield Text(self.html.rstrip("\\n"))', @@ -135,13 +135,13 @@ def test_an_html_comment_is_not_shown(self): @kills( Mutation( - console_module, + console_module._ChangelogMarkdown, ' elements: ClassVar = {**Markdown.elements, "html_block": _RawHtml}', " elements: ClassVar = {**Markdown.elements}", "a raw HTML block is dropped, taking the changes a `
` section wraps with it", ), Mutation( - console_module, + console_module._RawHtml.__rich_console__, ' yield Text(self.html.rstrip("\\n"))', " yield Text(self.html)", "a raw HTML block keeps the newline ending it, so a blank line follows every one", @@ -176,7 +176,7 @@ def test_changes_opening_with_a_list_a_quote_or_a_table_get_no_leading_line_brea @kills( Mutation( - console_module, + console_module.configure_logging, " logging.getLogger(_MARKDOWN_PARSER).setLevel(WARNING)\n return handler", " return handler", "the Markdown parser traces every block rule it tries, burying the run's own debug output", @@ -246,7 +246,7 @@ def test_a_shortcode_in_a_raw_html_block_renders_as_its_emoji(self): @kills( Mutation( - console_module, + console_module._ChangelogMarkdown.__init__, "super().__init__(markup, hyperlinks=hyperlinks)", "super().__init__(Emoji.replace(markup), hyperlinks=hyperlinks)", "replacing before the parse eats the shortcodes a code span and a fenced block hold", @@ -261,7 +261,7 @@ def test_a_shortcode_renders_as_its_emoji_except_inside_code(self): @kills( Mutation( - console_module, + console_module._ChangelogHandler.render_message, " return Group(rendered, Text(note))", " return Group(rendered, self._rendered_changes(note))", "Update-time's own note about a changelog is boxed as if it were a changelog's changes", @@ -287,7 +287,7 @@ def test_changes_that_are_not_markdown_render_as_written(self): @kills( Mutation( - console_module, + console_module._ChangelogHandler.render_message, " rendered = super().render_message(record, message)", " rendered = Markdown(super().render_message(record, message).plain)", "the message is rendered as Markdown too, mangling a command's stderr", diff --git a/tests/update_time/io/test_log.py b/tests/update_time/io/test_log.py index 21b26bc..e26a5c6 100644 --- a/tests/update_time/io/test_log.py +++ b/tests/update_time/io/test_log.py @@ -200,7 +200,7 @@ def test_pinned(self, mock_log: Mock): @kills( Mutation( - log_module, + log_module.Logger, ' "Floating tag %(dependency)s%(tag)s in %(location)s was left as it is: %(reason)s",', ' "Floating tag %(dependency)s%(tag)s in %(location)s was left as it is",', "the line reports that a tag was left as it is without naming the reason it was left", @@ -220,7 +220,7 @@ def test_unpinned_floating_tag(self, mock_log: Mock): @kills( Mutation( - log_module, + log_module.Logger, ' "Reference %(dependency)s%(tag)s in %(location)s was left as it is: %(reason)s",\n', ' "Reference %(dependency)s%(tag)s in %(location)s was left as it is",\n', "the line reports that a reference was left as it is without naming what accounts for it", @@ -252,7 +252,7 @@ def test_keeping_a_floating_tag(self, mock_log: Mock): @kills( Mutation( - dependency_module, + dependency_module.tag_of, ' return f":{version}" if version else ""', ' return f":{version}"', "a reference naming no tag is reported with a colon that names nothing after it", @@ -363,7 +363,7 @@ def test_yanked_dependency_warning(self, mock_log: Mock): @kills( Mutation( - log_module, + log_module.Logger._archival_fields, ' reason = f\' ("{archival.reason}")\' if archival.reason else ""', ' reason = ""', "the reason the source published is left out of the warning", @@ -566,7 +566,7 @@ def test_report_yank_does_nothing_when_not_yanked(self, mock_log: Mock): @kills( Mutation( - log_module, + log_module.Logger, ' _MESSAGE_IGNORED_VULNERABILITY = LogMessage(DEBUG, _ignoring("the %(advisory)s ' 'vulnerability warning"))', ' _MESSAGE_IGNORED_VULNERABILITY = LogMessage(DEBUG, _ignoring("the %(advisories)s ' @@ -589,7 +589,7 @@ def test_ignored_vulnerability(self, mock_log: Mock): @kills( Mutation( - log_module, + log_module.Logger, ' DEBUG, _ignoring("the %(advisory)s vulnerability warning", ' '"--ignore-vulnerability %(identifiers)s")', ' DEBUG, _ignoring("the %(identifiers)s vulnerability warning", ' @@ -788,7 +788,7 @@ def holders_by_template(cls) -> dict[str, set[str]]: @kills( Mutation( - log_module, + log_module.Logger, ' _MESSAGE_NO_VERSION = LogMessage(ERROR, "No valid version found for %(dependency)s")', ' _MESSAGE_ORPHANED = LogMessage(ERROR, "Nothing emits this")\n\n' ' _MESSAGE_NO_VERSION = LogMessage(ERROR, "No valid version found for %(dependency)s")', diff --git a/tests/update_time/manifests/test_pom_xml.py b/tests/update_time/manifests/test_pom_xml.py index 6a46c54..43ed516 100644 --- a/tests/update_time/manifests/test_pom_xml.py +++ b/tests/update_time/manifests/test_pom_xml.py @@ -64,11 +64,16 @@ """ -_POM_WITH_AN_INCOMPLETE_DEPENDENCY = """ +_POM_WITH_INCOMPLETE_DEPENDENCIES = """ spring-core + + org.springframework + + 6.1.0 + com.google.guava guava @@ -109,16 +114,22 @@ class DependenciesTest(unittest.TestCase): @kills( Mutation( - pom_xml, - " if not group_name or artifact is None or version is None:\n return None\n", + pom_xml._reference, + " if not group_name or not artifact_name or version is None:\n return None\n", "", "an element missing a part takes the whole pom's reading down with it", raises="AttributeError: 'NoneType' object has no attribute 'text'", - ) + ), + Mutation( + pom_xml._reference, + "not artifact_name or", + "artifact is None or", + "an empty artifact is read as the artefact `groupId:`, which Maven Central is then asked about", + ), ) def test_a_dependency_missing_a_part_maven_names_it_by(self): """Test that a dependency element missing a part is left out, and the pom is read all the same.""" - declared = pom_xml.dependencies(mock_path(_POM_WITH_AN_INCOMPLETE_DEPENDENCY)) or [] + declared = pom_xml.dependencies(mock_path(_POM_WITH_INCOMPLETE_DEPENDENCIES)) or [] self.assertEqual([reference.dependency for reference in declared], ["com.google.guava:guava"]) def test_a_dependency_naming_the_projects_own_group(self): diff --git a/tests/update_time/manifests/test_pyproject_toml.py b/tests/update_time/manifests/test_pyproject_toml.py index 5928899..6c89a9e 100644 --- a/tests/update_time/manifests/test_pyproject_toml.py +++ b/tests/update_time/manifests/test_pyproject_toml.py @@ -102,7 +102,7 @@ def test_bumps_known_versions(self): @kills( Mutation( - pyproject_toml, + pyproject_toml._rewritten_spec, ' return re.sub(rf"(==\\s*){re.escape(current)}", lambda match: match[1] + new_version, spec, count=1)', ' return f"{declaration.dependency}=={new_version}"', "a rewrite replaces the whole declaration, dropping the extra, environment marker, and spaces around " @@ -131,7 +131,7 @@ def test_leaves_a_matching_string_outside_the_dependency_arrays(self): @kills( Mutation( - pyproject_toml, + pyproject_toml._replaced_specs, " array[index] = toml.string(new_spec, quoted_as=spec)", " array[index] = new_spec", "a rewritten spec is quoted the way tomlkit quotes a plain string, so the file's own quoting is lost", @@ -222,7 +222,7 @@ def test_reads_a_block_that_is_never_closed_as_toml_throughout(self): @kills( Mutation( - pyproject_toml, + pyproject_toml._pinned_version, ' if len(specifiers) == 1 and specifiers[0].operator == "==" and is_valid(specifiers[0].version):', ' if (exact := [s for s in specifiers if s.operator == "=="]) and is_valid(exact[0].version):', "a declaration combining an equals with another specifier is read as a pin on the version the equals names", @@ -281,14 +281,14 @@ def test_a_pin_with_a_uv_source_is_read(self): @kills( Mutation( - pyproject_toml, + pyproject_toml.declared_dependencies, " uv_sourced=normalized_python_name(requirement.name) in sourced,", " uv_sourced=requirement.name in sourced,", "a dependency spelled another way than its `sources` key reads as one PyPI serves, so PyPI is asked " "about a name it has no release for", ), Mutation( - pyproject_toml, + pyproject_toml._uv_source_names, ' return {normalized_python_name(name) for name in _uv_table(config).get("sources", {})}', ' return set(_uv_table(config).get("sources", {}))', "a `sources` key spelled another way than the dependency it names covers it not, so PyPI is asked about " @@ -329,7 +329,7 @@ def test_reads_the_marker_a_declaration_spells_on_its_own_line(self): @kills( Mutation( - pyproject_toml, + pyproject_toml._marker, "parse_marker(replace(line, location=location))", 'parse_marker(replace(line, previous_text="", location=location))', "only an inline marker is read, so a marker on the line above a declaration steers nothing", @@ -359,7 +359,7 @@ def test_reads_the_marker_of_a_declaration_below_a_line_separator(self): @kills( Mutation( - pyproject_toml, + pyproject_toml.declared_dependencies, "file.toml(placed)", "placed", "a marker's lines come from the file, so every line of a `# /// script` block reads as a comment and " @@ -420,7 +420,7 @@ def test_reads_a_name_pinned_more_than_once(self): @kills( Mutation( - pyproject_toml, + pyproject_toml.declared_dependencies, " requirement.name,\n", ' requirement.name + "".join(f"[{extra}]" for extra in requirement.extras),\n', "a pin's name carries the extra its declaration spells, so it names no package the source knows", @@ -446,7 +446,7 @@ def test_reads_a_pin_with_an_extra_an_environment_marker_or_spaces(self): @kills( Mutation( - pyproject_toml, + pyproject_toml._pinned_version, ' if len(specifiers) == 1 and specifiers[0].operator == "==" and is_valid(specifiers[0].version):', ' if len(specifiers) == 1 and specifiers[0].operator == "==" ' 'and re.fullmatch("[0-9.]+", specifiers[0].version):', diff --git a/tests/update_time/references/test_file.py b/tests/update_time/references/test_file.py index 9c22aed..b46f78b 100644 --- a/tests/update_time/references/test_file.py +++ b/tests/update_time/references/test_file.py @@ -69,13 +69,13 @@ def update(self, mock_logger: Mock, get_new_version_for: Callable[[object], NewV @kills( Mutation( - file_module, + file_module.update_yaml_files, " else:\n", " if True:\n", "a file that does not parse is rewritten anyway, having been reported", ), Mutation( - file_module, + file_module.update_yaml_files, " logger.invalid_file(path, yaml_format.FORMAT)\n", " logger.invalid_file(path, yaml_format.FORMAT)\n return\n", "a file that does not parse ends the walk, so the files after it are never updated", @@ -94,7 +94,7 @@ def test_file_that_does_not_parse_is_reported_and_skipped(self, mock_glob: Mock) @kills( Mutation( - file_module, + file_module.update_yaml_files, " if document is yaml_format.UNPARSABLE:\n", " if document is yaml_format.UNPARSABLE or document is None:\n", "a file that parses to nothing is reported as invalid, the empty document reading as unparsable", diff --git a/tests/update_time/references/test_resolve.py b/tests/update_time/references/test_resolve.py index 93d8b10..647c23f 100644 --- a/tests/update_time/references/test_resolve.py +++ b/tests/update_time/references/test_resolve.py @@ -291,7 +291,7 @@ def test_warns_about_a_redundant_floating_pin_directive(self): @kills( Mutation( - resolve, + resolve._warn_if_the_floating_pin_is_redundant, " floats = None if latest is None else latest.floating is not None", ' floats = None if latest is None else latest.floating == "resolved"', "a floating pin the source could not resolve is reported as redundant, although its tag still floats", @@ -314,7 +314,7 @@ def test_no_redundant_floating_pin_directive_for_a_pin_the_source_could_not_reso @kills( Mutation( - resolve, + resolve.floating_pin_redundancy, "if not marker.allows(Scope.FLOATING_PIN):", "if not marker.allows(Scope.FLOATING_PIN) and Scope.FLOATING_PIN not in marker.written_scopes:", "the explicit default is reported as redundant, as if it kept the pin floating", @@ -386,7 +386,7 @@ def test_reports_both_checks_for_the_project(self): @kills( Mutation( - resolve, + resolve.report_project_checks, " if not project_is_checked(get_project, reference.dependency, threshold):\n return\n", "", "a source is asked about a reference no check needs an answer for, so the run pays for the request", @@ -403,7 +403,7 @@ def test_a_source_reporting_no_archival_is_not_asked_with_the_staleness_check_sw @kills( Mutation( - resolve, + resolve.project_is_checked, " return threshold != NO_STALENESS_CHECK or " "(archival_is_checked() and reports_archival(source, dependency))", " return threshold != NO_STALENESS_CHECK or reports_archival(source, dependency)", diff --git a/tests/update_time/references/test_rewrite.py b/tests/update_time/references/test_rewrite.py index 3295e0e..c650a99 100644 --- a/tests/update_time/references/test_rewrite.py +++ b/tests/update_time/references/test_rewrite.py @@ -371,7 +371,7 @@ def test_drift_adopted_for_a_tag_kept_floating(self): @kills( Mutation( - marker_module, + marker_module.Marker.allow_directive, ' return _directive(Verb.ALLOW, str(scope)) if self.allows(scope) else ""', ' return self.raw_directives(Verb.ALLOW) if self.allows(scope) else ""', "the cause names every allow directive again, not the one that kept the tag floating", diff --git a/tests/update_time/sources/test_docker_hub.py b/tests/update_time/sources/test_docker_hub.py index 58098ce..dc33238 100644 --- a/tests/update_time/sources/test_docker_hub.py +++ b/tests/update_time/sources/test_docker_hub.py @@ -46,7 +46,7 @@ def test_no_headers_when_token_request_fails(self): @patch_environ({"DOCKER_HUB_USERNAME": "joe_doe", "DOCKER_HUB_TOKEN": "pat123"}) # nosec @kills( Mutation( - docker_hub, + docker_hub.api_headers, 'response.json().get("access_token")', 'response.json()["access_token"]', "a Docker Hub token response carrying no token ends the run with a traceback", diff --git a/tests/update_time/sources/test_github.py b/tests/update_time/sources/test_github.py index 0c32e01..db80369 100644 --- a/tests/update_time/sources/test_github.py +++ b/tests/update_time/sources/test_github.py @@ -24,6 +24,7 @@ changes_from_release, github_owner_and_repository, github_to_raw, + release_tags, ) from tests.helpers import mock_response, patch_environ, patch_get @@ -103,6 +104,18 @@ def test_github_url(self): """Test that a GitHub URL returns an owner and repository.""" self.assert_owner_and_repository(("ICTU", "quality-time"), "https://github.com/ICTU/quality-time") + @kills( + Mutation( + github.github_owner_and_repository, + " if len(path_parts) > 1 and path_parts[0] != _GITHUB_SPONSORS_PATH:", + " if len(path_parts) > 1:", + "a GitHub sponsors page is read as the repository `sponsors/`, which GitHub is then asked about", + ) + ) + def test_github_sponsors_url(self): + """Test that a GitHub sponsors URL returns an empty owner and repository.""" + self.assert_owner_and_repository(("", ""), "https://github.com/sponsors/ICTU") + def test_github_url_without_repo(self): """Test that a GitHub URL returns an empty owner and repository if the repository is missing.""" self.assert_owner_and_repository(("", ""), "https://github.com/ICTU") @@ -218,7 +231,7 @@ def test_no_commit_sha(self): @kills( Mutation( - github, + github.get_latest_version, " return replace(latest, project=repository_project)", " return replace(latest, project=Project(newest=replace(repository_project.newest, " "version=latest.version) if repository_project.newest else None))", @@ -300,7 +313,7 @@ def test_skip_tag_whose_commit_has_no_date(self): @kills( Mutation( - github, + github._commit_datetime, 'return parse_timestamp(committer.get("date")) if committer else None', 'return parse_timestamp(committer.get("date"))', "a commit whose committer GitHub reports as null ends the run with a traceback", @@ -343,7 +356,7 @@ class NewestReleaseTest(LoggingTestCase): @kills( Mutation( - dependency, + dependency.Release.__lt__, " return (self.published, self._sortable_version) < (other.published, other._sortable_version)", " return (self._sortable_version, self.published) < (other._sortable_version, other.published)", "the release named is the highest version rather than the one whose date was measured", @@ -391,7 +404,7 @@ def test_tag_running_ahead_of_releases(self): @kills( Mutation( - github, + github.TaggedVersion.version_string, " return str(self.version) if self.has_valid_version else self.tag_name", " return str(self.version)", "a release tagged with something that is no version ends the run with a traceback", @@ -415,7 +428,7 @@ class ArchivalTest(LoggingTestCase): @kills( Mutation( - github, + github._repository_metadata, ' response = _fetch_github(f"{_GITHUB_API}/{owner}/{repository}")', ' response = _fetch_github(f"{_GITHUB_API}/{owner}/{repository}/")', "the repository is asked for at a URL GitHub answers 404, so no repository ever reads as archived", @@ -472,7 +485,7 @@ def assert_releases(self, repository: str, cases: list[tuple[str, str, str]]) -> """Assert that each case's package and version resolve, in the repository, to the release its tag names.""" for package, version, tag in cases: with self.subTest(tag=tag): - self.assert_release(_get_release("owner", repository, package, version), tag, "Changelog") + self.assert_release(_get_release("owner", repository, release_tags(package, version)), tag, "Changelog") @patch_get( [ @@ -494,25 +507,23 @@ def test_monorepo_tag_match(self): @kills( Mutation( - github, + github._package_names, "[package] if unscoped == package else [package, unscoped]", "[package]", "a scoped npm package tagged without its scope has no changelog reported", raises="AttributeError: 'NoneType' object has no attribute 'tag_name'", ), Mutation( - github, - 'for name in _package_names(package) for joiner in ("-v", "-", "@", "/")', - 'for name in _package_names(package) for joiner in ("-v", "-", "@", "/") ' - 'if name == package or joiner == "@"', + github.release_tags, + 'for name in names for joiner in ("-v", "-", "@", "/")', + 'for name in names for joiner in ("-v", "-", "@", "/") if name == package or joiner == "@"', "a monorepo prefixing the unscoped name with a dash has no changelog reported", raises="AttributeError: 'NoneType' object has no attribute 'tag_name'", ), Mutation( - github, - 'for name in _package_names(package) for joiner in ("-v", "-", "@", "/")', - 'for name in _package_names(package) for joiner in ("-v", "-", "@", "/") ' - 'if name == package or joiner != "/"', + github.release_tags, + 'for name in names for joiner in ("-v", "-", "@", "/")', + 'for name in names for joiner in ("-v", "-", "@", "/") if name == package or joiner != "/"', "a monorepo joining the unscoped name and the version with a slash has no changelog reported", raises="AttributeError: 'NoneType' object has no attribute 'tag_name'", ), @@ -538,7 +549,7 @@ def test_scoped_tag_without_its_scope_match(self): @kills( Mutation( - github, + github._package_names, "[package] if unscoped == package else [package, unscoped]", "[package] if unscoped == package else [unscoped, package]", "a repository tagging one version both with and without the scope has the wrong release reported for it", @@ -552,14 +563,14 @@ def test_scoped_tag_without_its_scope_match(self): ) def test_scoped_tag_takes_precedence(self): """Test that the tag carrying the scope wins over the one spelling the same version without it.""" - release = _get_release("emotion-js", "emotion", "@emotion/react", "11.14.0") + release = _get_release("emotion-js", "emotion", release_tags("@emotion/react", "11.14.0")) self.assert_release(release, "@emotion/react@11.14.0", "Scoped") @kills( Mutation( - github, - 'for tag in [*package_tags, f"v{version}", version]:', - 'for tag in [*package_tags[:1], f"v{version}", version, *package_tags[1:]]:', + github.release_tags, + 'return [*package_tags, f"v{version}", version]', + 'return [*package_tags[:1], f"v{version}", version, *package_tags[1:]]', "a package whose repository also tags the version alone has the wrong release reported for it", ), ) @@ -581,7 +592,7 @@ def test_package_tag_takes_precedence(self): @kills( Mutation( - github, + github.release_tags, '("-v", "-", "@", "/")', '("-", "-v", "@", "/")', "a repository spelling one version's tag both ways has the wrong release reported for it", @@ -597,7 +608,9 @@ def test_package_tag_takes_precedence(self): ) def test_monorepo_tag_takes_precedence(self): """Test that the package-prefixed tag with a v wins over the other spellings of the same version.""" - self.assert_release(_get_release("puppeteer", "monorepo", "puppeteer-core", "25.0.4"), "puppeteer-core-v25.0.4") + self.assert_release( + _get_release("puppeteer", "monorepo", release_tags("puppeteer-core", "25.0.4")), "puppeteer-core-v25.0.4" + ) @patch_get([github_release_json("v1.2.3", body="Changelog"), github_release_json("4.5.6", body="Changelog")]) def test_version_tag_match(self): @@ -608,20 +621,20 @@ def test_version_tag_match(self): @patch_get([github_release_json("v1.0")]) def test_no_matching_tag(self): """Test that None is returned when no tag matches the requested version.""" - self.assertIsNone(_get_release("owner", "repo with non matching tag", "any", "1.1")) + self.assertIsNone(_get_release("owner", "repo with non matching tag", release_tags("any", "1.1"))) @patch("requests.get") def test_repo_without_releases(self, mock_get: Mock): """Test that a non-OK response yields no release, and is reported as a failed fetch.""" mock_get.return_value = mock_response([], ok=False) - self.assertIsNone(_get_release("owner", "repo without releases for get_release", "any", "1.0")) + self.assertIsNone(_get_release("owner", "repo without releases for get_release", release_tags("any", "1.0"))) self.assert_could_not_fetch_logged(mock_get().url, mock_get().status_code) @patch("requests.get") def test_timeout(self, mock_get: Mock): """Test that a timed-out request yields no release, and is reported as a timeout.""" mock_get.side_effect = requests.exceptions.Timeout - self.assertIsNone(_get_release("owner", "repo without releases for get_release", "any", "1.0")) + self.assertIsNone(_get_release("owner", "repo without releases for get_release", release_tags("any", "1.0"))) url = "https://api.github.com/repos/owner/repo without releases for get_release/releases?per_page=100" self.assert_logged(Logger._MESSAGE_TIMEOUT, url=url) @@ -637,7 +650,7 @@ def test_no_owner_or_repository(self, mock_get: Mock): @kills( Mutation( - github, + github.TaggedVersion.from_release, 'body=Changes(release.get("body") or "", markdown=True),', 'body=Changes(release.get("body") or "", markdown=False),', "a release body is read as text, so the Markdown GitHub renders it in is shown raw", @@ -657,7 +670,7 @@ def test_no_matching_release(self): @kills( Mutation( - github, + github.TaggedVersion.from_release, 'body=Changes(release.get("body") or "", markdown=True),', 'body=Changes(release["body"] or "", markdown=True),', "a release GitHub answers without a body ends the run with a traceback", @@ -691,7 +704,7 @@ def create_responses(self, mock_get: Mock, listings: dict[str, tuple[str, ...]]) @kills( Mutation( - github, + github.changes_from_changelog_file, " if directory:", " _list_contents(owner, repository)\n if directory:", "a package whose own directory holds its changelog costs a listing of the repository's root as well", @@ -715,7 +728,7 @@ def test_directory_without_a_changelog_file(self, mock_get: Mock): @kills( Mutation( - github, + github._changes_from_files, ' return Changes(changes, markdown=is_markdown_file(entry["name"]))', " return Changes(changes, markdown=False)", "a Markdown changelog in a repository's root is read as text, so its markup is shown raw", @@ -754,7 +767,7 @@ def test_changelog_file_is_fetched_once(self, mock_get: Mock): @kills( Mutation( - github, + github._list_contents, ' return _list(owner, repository, f"contents/{directory}", require_ok=not directory)', ' return _list(owner, repository, f"contents/{directory}")', "a package whose registry metadata names a directory that moved warns on every run", @@ -778,7 +791,7 @@ def test_directory_the_repository_does_not_serve(self, mock_get: Mock): @kills( Mutation( - github, + github._list, " listing = response.json()\n return tuple(listing) if isinstance(listing, list) else ()", " return tuple(response.json())", "a directory the contents endpoint answers with a file ends the run with a traceback", @@ -815,7 +828,7 @@ def assert_request_headers(self, mock_get: Mock, expected: dict[str, str]) -> No @kills( Mutation( - github, + github._github_headers, '{"Authorization": f"Bearer {github_token}"}', "{}", "every GitHub API request goes out unauthenticated, so a run spends an anonymous caller's rate limit", @@ -829,7 +842,7 @@ def test_authorization_header_when_a_token_is_set(self, mock_get: Mock): @kills( Mutation( - github, + github._github_headers, '("GITHUB_TOKEN")) else {}', '("GITHUB_TOKEN")) else {"Authorization": "Bearer"}', "a run given no token sends an empty bearer header instead of asking anonymously", diff --git a/tests/update_time/sources/test_maven_central.py b/tests/update_time/sources/test_maven_central.py index 93ea478..572bc96 100644 --- a/tests/update_time/sources/test_maven_central.py +++ b/tests/update_time/sources/test_maven_central.py @@ -12,12 +12,19 @@ from update_time.io.log import Logger from update_time.manifests import pom_xml as pom_xml_format from update_time.sources import maven_central -from update_time.sources.maven_central import _newest_release, _published, project, versions_within_cooldown +from update_time.sources.maven_central import ( + _newest_release, + _published, + get_changes, + project, + versions_within_cooldown, +) from tests.helpers import mock_response, patch_environ, patch_get from tests.mutation import Mutation, kills from tests.update_time.helpers import ( LoggingTestCase, + github_release_json, maven_central_dated_row, maven_central_listing, maven_central_pom, @@ -25,14 +32,36 @@ maven_central_version_row, patch_maven_central, ) -from tests.update_time.sources.helpers import requested_urls +from tests.update_time.sources.helpers import ( + contents_json, + contents_url, + file_url, + markdown_changelog, + markdown_changes, + releases_url, + requested_urls, + respond_per_url, +) + + +def _guava_pom_url(artifact: str) -> str: + """Return the URL of the pom Maven Central serves for the artifact in guava's group, at guava's version.""" + return f"{_GUAVA_GROUP_URL}/{artifact}/{_GUAVA_VERSION}/{artifact}-{_GUAVA_VERSION}.pom" + _GUAVA = "com.google.guava:guava" -_GUAVA_LISTING = "https://repo1.maven.org/maven2/com/google/guava/guava/" +_GUAVA_GROUP_URL = "https://repo1.maven.org/maven2/com/google/guava" +_GUAVA_LISTING = f"{_GUAVA_GROUP_URL}/guava/" _GUAVA_VERSION = "33.7.1-jre" -_GUAVA_POM = f"{_GUAVA_LISTING}{_GUAVA_VERSION}/guava-{_GUAVA_VERSION}.pom" +_GUAVA_POM = _guava_pom_url("guava") _GUAVA_REPOSITORY = "https://api.github.com/repos/google/guava" _GUAVA_SCM = "scm:git:https://github.com/google/guava.git" +_GUAVA_PARENT = f"com.google.guava:guava-parent:{_GUAVA_VERSION}" +_GUAVA_PARENT_POM = _guava_pom_url("guava-parent") +# An artefact whose repository is named otherwise than its artifact. +_NETTY_ALL = "io.netty:netty-all" +_NETTY_VERSION = "4.1.138.Final" +_NETTY_SCM = "scm:git:https://github.com/netty/netty.git" def _days_ago(days: int) -> datetime: @@ -53,9 +82,24 @@ def _metadata_rows(published: datetime) -> tuple[str, ...]: return tuple(maven_central_row(name, published, size) for name, size in _METADATA_FILES.items()) +def _listing_of(version: str) -> str: + """Return a listing that holds the version alone, dated yesterday.""" + return maven_central_listing(maven_central_version_row(version, _days_ago(1))) + + # The listing the repository serves for guava where the tests care about the pom beside a version rather than the -# dates of the versions themselves: one version, dated yesterday. -_DATED_LISTING = maven_central_listing(maven_central_version_row(_GUAVA_VERSION, _days_ago(1))) +# dates of the versions themselves. +_DATED_LISTING = _listing_of(_GUAVA_VERSION) + + +def _serve_guava_with_parent(mock_get: Mock, parent_pom: str, responses: dict[str, Mock] | None = None) -> None: + """Point the mock requests.get at guava's listing, a guava pom naming guava's parent, and the parent's pom.""" + served = { + _GUAVA_LISTING: mock_response(text=_DATED_LISTING), + _GUAVA_POM: mock_response(content=maven_central_pom(parent=_GUAVA_PARENT).encode()), + _GUAVA_PARENT_POM: mock_response(content=parent_pom.encode()), + } + respond_per_url(mock_get, served | (responses or {})) class ProjectTest(LoggingTestCase): @@ -97,15 +141,105 @@ def test_a_pom_naming_an_archived_repository(self): with patch_maven_central(_DATED_LISTING, served, archived=True) as mock_get: self.assert_archived(mock_get, project(_GUAVA, check_archival=True).archival) + @kills( + Mutation( + maven_central._pom_repository, + " named = _named_repository(pom)\n", + " named = _named_repository(pom)\n" + " if eager := pom_xml_format.parent(pom):\n" + " _pom(eager.name, eager.version)\n", + "every artefact whose pom names its repository costs a request for its parent pom as well", + ) + ) + def test_a_pom_naming_a_github_repository_leaves_its_parent_pom_unread(self): + """Test that an artefact whose pom names a GitHub repository is checked without a request for its parent pom.""" + served = maven_central_pom(_GUAVA_SCM, parent=_GUAVA_PARENT) + with patch_maven_central(_DATED_LISTING, served, archived=True) as mock_get: + self.assert_archived(mock_get, project(_GUAVA, check_archival=True).archival) + + @kills( + Mutation( + maven_central._parent_pom, + "parent = pom_xml_format.parent(pom)", + "parent = None", + "an artefact whose pom leaves its repository to the parent pom goes unchecked for archival", + ) + ) + def test_a_pom_naming_no_repository_takes_the_one_its_parent_pom_names(self): + """Test that an artefact whose own pom does not name a GitHub repository is checked against its parent's.""" + with patch("requests.get") as mock_get: + archived = {_GUAVA_REPOSITORY: mock_response({"archived": True})} + _serve_guava_with_parent(mock_get, maven_central_pom(_GUAVA_SCM), archived) + archival = project(_GUAVA, check_archival=True).archival + self.assertEqual(archival, Archival(archived=True, subject=ArchivedSubject.REPOSITORY)) + self.assertEqual(requested_urls(mock_get), [_GUAVA_LISTING, _GUAVA_POM, _GUAVA_PARENT_POM, _GUAVA_REPOSITORY]) + + @kills( + Mutation( + maven_central._pom_repository, + "_parent_pom(pom))", + "_parent_pom(_parent_pom(pom) or pom))", + "an artefact's grandparent pom is read too, which may name a generic parent project's repository", + raises=f"KeyError: '{_guava_pom_url('guava-grandparent')}'", + ) + ) + def test_a_parent_pom_naming_no_repository_leaves_its_own_parent_unread(self): + """Test that a parent pom that does not name a GitHub repository leaves GitHub and its own parent unasked.""" + parent_pom = maven_central_pom(parent=f"com.google.guava:guava-grandparent:{_GUAVA_VERSION}") + with patch("requests.get") as mock_get: + _serve_guava_with_parent(mock_get, parent_pom) + archival = project(_GUAVA, check_archival=True).archival + self.assert_github_unasked(mock_get, archival, _GUAVA_LISTING, _GUAVA_POM, _GUAVA_PARENT_POM) + + @kills( + Mutation( + pom_xml_format.fully_resolved, + "part and _is_resolved(part)", + "_is_resolved(part)", + "a parent pom is fetched at a URL with a coordinate left out, which Maven Central serves nothing at", + ), + Mutation( + pom_xml_format.parent, + " return pinned if fully_resolved(pinned) else None", + " return pinned", + "a parent pom is fetched at a URL that spells out a property, which Maven Central serves nothing at", + ), + ) + def test_a_parent_missing_a_coordinate_or_naming_one_by_property_is_passed_over(self): + """Test that a `` without a group, artifact, or version of its own leaves its pom and GitHub unasked.""" + cases = { + "no groupId": f":guava-parent:{_GUAVA_VERSION}", + "no artifactId": f"com.google.guava::{_GUAVA_VERSION}", + "no version": "com.google.guava:guava-parent:", + "groupId as a property": f"${{project.groupId}}:guava-parent:{_GUAVA_VERSION}", + "version as a property": "com.google.guava:guava-parent:${revision}", + } + for case, parent in cases.items(): + with self.subTest(case=case): + self.clear_caches() + served = maven_central_pom(parent=parent) + with patch_maven_central(_DATED_LISTING, served, archived=True) as mock_get: + archival = project(_GUAVA, check_archival=True).archival + self.assert_github_unasked(mock_get, archival, _GUAVA_LISTING, _GUAVA_POM) + + @kills( + Mutation( + maven_central._parent_pom, + " if pom_xml_format.scm_urls(pom):\n return None\n", + "", + "an artefact whose pom names its repository on another host is given its generic parent's repository", + ) + ) def test_a_pom_naming_a_repository_on_another_host(self): - """Test that an artefact whose pom names a repository outside GitHub leaves GitHub unasked.""" + """Test that an artefact whose pom names a repository outside GitHub leaves its parent and GitHub unasked.""" elsewhere = "scm:git:https://gitbox.apache.org/repos/asf/commons-lang.git" - with patch_maven_central(_DATED_LISTING, maven_central_pom(elsewhere), archived=True) as mock_get: + served = maven_central_pom(elsewhere, parent=_GUAVA_PARENT) + with patch_maven_central(_DATED_LISTING, served, archived=True) as mock_get: archival = project(_GUAVA, check_archival=True).archival self.assert_github_unasked(mock_get, archival, _GUAVA_LISTING, _GUAVA_POM) def test_a_pom_naming_no_source_repository(self): - """Test that an artefact whose pom declares no `` element leaves GitHub unasked.""" + """Test that a pom naming a repository in neither `` nor its `` leaves GitHub unasked.""" with patch_maven_central(_DATED_LISTING, maven_central_pom(), archived=True) as mock_get: archival = project(_GUAVA, check_archival=True).archival self.assert_github_unasked(mock_get, archival, _GUAVA_LISTING, _GUAVA_POM) @@ -127,11 +261,10 @@ def test_a_pom_the_repository_does_not_serve(self): @kills( Mutation( - pom_xml_format.scm_urls, - " if project is None:\n return None\n", - "", - "an artefact pom whose XML does not parse ends the run", - raises="AttributeError: 'NoneType' object has no attribute 'child'", + maven_central._pom, + "_LOG.invalid_pom(url)", + "pass", + "an artefact pom whose XML does not parse goes unreported", ) ) def test_a_pom_whose_xml_does_not_parse(self): @@ -144,8 +277,8 @@ def test_a_pom_whose_xml_does_not_parse(self): @kills( Mutation( maven_central, - "@cache\ndef _pom", - "def _pom", + "@cache\ndef _pom(", + "def _pom(", "every pom declaring an artefact costs a pom request of its own", ) ) @@ -158,6 +291,21 @@ def test_an_artefact_is_asked_for_its_pom_once_per_run(self): self.assert_archived(mock_get, first) self.assertEqual(second, first) + @kills( + Mutation( + maven_central._parent_pom, + "_pom(parent.name", + "_pom.__wrapped__(parent.name", + "every artefact sharing a parent costs a request for the parent's pom of its own", + ) + ) + def test_artefacts_sharing_a_parent_are_asked_for_its_pom_once_per_run(self): + """Test that two artefacts whose poms name the same parent cost one request for the parent's pom.""" + with patch_maven_central(_DATED_LISTING, maven_central_pom(parent=_GUAVA_PARENT)) as mock_get: + project(_GUAVA, check_archival=True) + project("com.google.guava:guava-testlib", check_archival=True) + self.assertEqual(requested_urls(mock_get).count(_GUAVA_PARENT_POM), 1) + def test_a_run_that_checks_nothing_for_archival(self): """Test that a run checking no dependency for archival reads no pom, so the listing is all it asks for.""" with patch_maven_central(_DATED_LISTING, maven_central_pom(_GUAVA_SCM), archived=True) as mock_get: @@ -165,6 +313,256 @@ def test_a_run_that_checks_nothing_for_archival(self): self.assert_github_unasked(mock_get, archival, _GUAVA_LISTING) +class GetChangesTest(LoggingTestCase): + """Unit tests for reading what a version of an artefact changed.""" + + @staticmethod + def serve_releases_and_changelog( + mock_get: Mock, releases: list[dict[str, object]], heading: str = _GUAVA_VERSION + ) -> None: + """Point the mock requests.get at guava's listing and pom, and at a repository holding the releases. + + The repository's root holds a changelog file with a section headed `heading`. Unlike `patch_maven_central`, + which answers any URL it does not know with the listing, this fails a request for a URL it does not serve. + """ + respond_per_url( + mock_get, + { + _GUAVA_LISTING: mock_response(text=_DATED_LISTING), + _GUAVA_POM: mock_response(content=maven_central_pom(_GUAVA_SCM).encode()), + releases_url("google/guava"): mock_response(releases), + contents_url("google/guava"): mock_response(contents_json("CHANGELOG.md")), + file_url("CHANGELOG.md"): mock_response(text=markdown_changelog(heading)), + }, + ) + + def assert_changes_from_releases(self, cases: dict[str, tuple[str, str]]) -> None: + """Assert that each case's version has the changes of the release the repository published under its tag.""" + for case, (version, tag) in cases.items(): + with self.subTest(case=case): + self.clear_caches() + releases = [github_release_json(tag, body=f"Changes in {version}")] + with patch_maven_central(_listing_of(version), maven_central_pom(_GUAVA_SCM), releases=releases): + self.assertEqual(get_changes(_GUAVA, version), f"Changes in {version}") + + @kills( + Mutation( + maven_central.get_changes, + "repository, version)\n", + "repository, _newest_release(artefact).version)\n", + "the newest release's changes are reported for a version the run left behind it", + ), + ) + def test_the_changes_are_those_of_the_release_matching_the_version(self): + """Test that the changes are the body of the release for the version the run left the dependency on.""" + releases = [ + github_release_json("v33.6.0-jre", body="Changes in 33.6.0"), + github_release_json(f"v{_GUAVA_VERSION}", body="Changes in 33.7.1"), + ] + with patch_maven_central(_DATED_LISTING, maven_central_pom(_GUAVA_SCM), releases=releases): + self.assertEqual(get_changes(_GUAVA, "33.6.0-jre"), "Changes in 33.6.0") + + @kills( + Mutation( + maven_central.get_changes, + "_repository(artefact)", + "_pom_repository(artefact, version)", + "the repository is read from the pom beside the version, which may predate the project's move to GitHub", + ) + ) + def test_the_repository_is_read_from_the_newest_releases_pom(self): + """Test that the pom naming the repository is the newest release's, whatever version the changes are for.""" + with patch_maven_central(_DATED_LISTING, maven_central_pom(_GUAVA_SCM)) as mock_get: + get_changes(_GUAVA, "33.6.0-jre") + self.assertIn(_GUAVA_POM, requested_urls(mock_get)) + + @kills( + Mutation( + maven_central._release_tags, + "(artifact_id,", + "(artefact,", + "a release tagged by the artifact's name goes unmatched, since the tag names never carry the group", + ), + Mutation( + maven_central._release_tags, + "spelling, repository)", + "spelling)", + "a release tagged by the repository's name rather than the artifact's goes unmatched", + ), + Mutation( + maven_central._release_tags, + "(artifact_id, spelling, repository)", + "(repository, spelling, artifact_id)", + "a repository tagging one version by both names has the repository's release reported for the artifact", + ), + Mutation( + maven_central._release_tags, + "*release_tags(artifact_id, spelling, repository),", + "*release_tags(artifact_id, spelling),\n *release_tags(repository, spelling),", + "a repository tagging one version by its name and by the version alone has the wrong release reported", + ), + Mutation( + maven_central._spellings, + '(version, _QUALIFIER.sub("", version))', + '(_QUALIFIER.sub("", version), version)', + "a repository tagging one version with and without its qualifier has the wrong release reported for it", + ), + ) + def test_the_more_specific_of_two_tags_for_a_version_takes_precedence(self): + """Test that the release under the more specific of two tags for the same version holds its changes.""" + cases = { + "artifact name over repository name": (f"netty-{_NETTY_VERSION}", f"netty-all-{_NETTY_VERSION}"), + "repository name over version alone": (f"v{_NETTY_VERSION}", f"netty-{_NETTY_VERSION}"), + "qualifier over no qualifier": ("v4.1.138", f"v{_NETTY_VERSION}"), + } + for case, (less_specific, more_specific) in cases.items(): + with self.subTest(case=case): + self.clear_caches() + releases = [ + github_release_json(less_specific, body="Less specific"), + github_release_json(more_specific, body="More specific"), + ] + with patch_maven_central(_listing_of(_NETTY_VERSION), maven_central_pom(_NETTY_SCM), releases=releases): + self.assertEqual(get_changes(_NETTY_ALL, _NETTY_VERSION), "More specific") + + @kills( + Mutation( + maven_central._release_tags, + "in _spellings(version)", + "in (version,)", + "a release tagged without the qualifier Maven appends to the version goes unmatched", + ) + ) + def test_a_release_tagged_with_the_version_without_its_qualifier_matches(self): + """Test that a release tagged with the version less the qualifier Maven appends holds the version's changes.""" + cases = {"dash qualifier": ("33.7.1-jre", "v33.7.1"), "dot qualifier": ("7.4.10.Final", "7.4.10")} + self.assert_changes_from_releases(cases) + + @kills( + Mutation( + maven_central._release_tags, + "in _VERSION_PREFIXES", + "in ()", + "a release tagged with a prefix other than `v` goes unmatched", + ), + Mutation( + maven_central._release_tags, + 'f"{prefix}{spelling}"', + 'f"{prefix}{version}"', + "a release tagged with a prefix other than `v` and without the version's qualifier goes unmatched", + ), + ) + def test_a_release_tagged_with_a_prefix_other_than_v_matches(self): + """Test that a release tagged with the version behind `r`, `version-`, or `REL` holds the version's changes.""" + cases = { + "r": ("6.1.3", "r6.1.3"), + "version-": ("2.5.250", "version-2.5.250"), + "REL": ("42.7.13", "REL42.7.13"), + "prefix without the qualifier": ("6.1.3-jre", "r6.1.3"), + } + self.assert_changes_from_releases(cases) + + @kills( + Mutation( + maven_central.get_changes, + " or _changes_from_changelog_file(\n owner, repository, version\n )", + "", + "the changes of a version its repository tagged without publishing a release go unreported", + ), + Mutation( + maven_central._changes_from_changelog_file, + "in _spellings(version)", + "in (version,)", + "a changelog heading the version without the qualifier Maven appends has its changes go unreported", + ), + ) + def test_the_changes_come_from_a_changelog_file_where_no_release_matches(self): + """Test that the changes come from the changelog file when the repository did not release the version.""" + for heading in (_GUAVA_VERSION, "33.7.1"): + with self.subTest(heading=heading), patch("requests.get") as mock_get: + self.clear_caches() + self.serve_releases_and_changelog(mock_get, [github_release_json("v33.6.0-jre")], heading) + self.assertEqual(get_changes(_GUAVA, _GUAVA_VERSION), markdown_changes(heading)) + + @kills( + Mutation( + maven_central.get_changes, + "return changes_from_tagged_release(", + "return changes_from_changelog_file(owner, repository, version) or changes_from_tagged_release(", + "the changelog file wins over the release the repository published for the version", + ), + Mutation( + maven_central.get_changes, + " tags =", + " changelog = changes_from_changelog_file(owner, repository, version)\n tags =", + "every dependency Maven moved costs a changelog file request, whether or not a release matched", + ), + ) + def test_a_matching_release_wins_over_the_changelog_file(self): + """Test that the changes come from the release matching the version, without reading the changelog file.""" + releases = [github_release_json(f"v{_GUAVA_VERSION}", body="Changes in 33.7.1")] + with patch("requests.get") as mock_get: + self.serve_releases_and_changelog(mock_get, releases) + self.assertEqual(get_changes(_GUAVA, _GUAVA_VERSION), "Changes in 33.7.1") + self.assertNotIn(contents_url("google/guava"), requested_urls(mock_get)) + + @kills( + Mutation( + pom_xml_format.source_urls, + " return urls if project_url is None else [*urls, project_url.text]", + " return urls", + "a pom naming its repository in the project's `` alone is read as naming none", + ), + Mutation( + pom_xml_format.source_urls, + " urls = scm_urls(project)\n", + ' urls = scm_urls(project)\n if project.child("scm") is None:\n return urls\n', + "Update-time reads the project's `` only when the pom declares an `` element", + ), + Mutation( + pom_xml_format.source_urls, + "[*urls, project_url.text]", + "[project_url.text, *urls]", + "the project's `` wins over ``, so a module's changes are read from its umbrella project", + ), + ) + def test_the_changes_come_from_the_first_repository_on_github_the_pom_names(self): + """Test that `` names the repository the changes come from, and the project's `` where it fails to.""" + umbrella = "https://github.com/google/guava-umbrella" + cases = { + "no scm": (maven_central_pom(project_url=umbrella), "google/guava-umbrella"), + "scm on another host": ( + maven_central_pom("scm:git:https://gitbox.example.org/guava.git", project_url=umbrella), + "google/guava-umbrella", + ), + "scm and url on github": (maven_central_pom(_GUAVA_SCM, project_url=umbrella), "google/guava"), + } + releases = [github_release_json(f"v{_GUAVA_VERSION}", body="Changes in 33.7.1")] + for case, (pom, repository) in cases.items(): + with self.subTest(case=case): + self.clear_caches() + with patch_maven_central(_DATED_LISTING, pom, releases=releases) as mock_get: + self.assertEqual(get_changes(_GUAVA, _GUAVA_VERSION), "Changes in 33.7.1") + self.assertIn(releases_url(repository), requested_urls(mock_get)) + + @kills( + Mutation( + maven_central.get_changes, + "_repository(artefact)", + "_named_repository(_pom(artefact, _newest_release(artefact).version))", + "an artefact whose pom leaves its repository to the parent pom has its changes go unreported", + ) + ) + def test_the_changes_come_from_the_repository_the_parent_pom_names(self): + """Test that an artefact whose own pom does not name a GitHub repository has its parent's release notes.""" + releases = [github_release_json(f"v{_GUAVA_VERSION}", body="Changes in 33.7.1")] + with patch("requests.get") as mock_get: + _serve_guava_with_parent( + mock_get, maven_central_pom(_GUAVA_SCM), {releases_url("google/guava"): mock_response(releases)} + ) + self.assertEqual(get_changes(_GUAVA, _GUAVA_VERSION), "Changes in 33.7.1") + + class VersionsWithinCooldownTest(LoggingTestCase): """Unit tests for reading the versions the repository published inside the cooldown window.""" @@ -177,7 +575,7 @@ def test_only_a_version_published_inside_the_window_is_held_back(self): @kills( Mutation( - maven_central, + maven_central._listing, " if response.status_code == HTTPStatus.NOT_FOUND:\n _LOG.unserved_listing(response)\n" ' return ""\n', "", @@ -199,7 +597,7 @@ def test_an_unserved_listing_holds_no_version_back(self): @kills( Mutation( - maven_central, + maven_central._listing, ' if not response.ok:\n _LOG.response(response)\n return ""\n', "", "the repository fails without a word, so the cooldown is lost for an artefact it does serve", @@ -217,7 +615,7 @@ def test_an_error_other_than_a_missing_listing_is_warned_about(self): @kills( Mutation( - maven_central, + maven_central._listing, ' if response is None:\n return ""\n', "", "a listing whose request fails ends the run with a traceback", @@ -232,7 +630,7 @@ def test_a_listing_request_that_times_out_holds_no_version_back(self): @kills( Mutation( - maven_central, + maven_central._published, " try:\n return datetime.strptime(published, _PUBLISHED_FORMAT).replace(tzinfo=UTC)\n" " except ValueError:\n return None\n", " return datetime.strptime(published, _PUBLISHED_FORMAT).replace(tzinfo=UTC)\n", @@ -291,7 +689,7 @@ class PublishedTest(unittest.TestCase): @kills( Mutation( - maven_central, + maven_central._published, " return datetime.strptime(published, _PUBLISHED_FORMAT).replace(tzinfo=UTC)\n", " return datetime.strptime(published, _PUBLISHED_FORMAT).astimezone()\n", "a date is read in the machine's own zone, so a version falls on the wrong side of the window", diff --git a/tests/update_time/sources/test_npmjs.py b/tests/update_time/sources/test_npmjs.py index 1324947..368addc 100644 --- a/tests/update_time/sources/test_npmjs.py +++ b/tests/update_time/sources/test_npmjs.py @@ -55,7 +55,7 @@ def test_get_publication_datetime_when_unreachable(self): @kills( Mutation( - npmjs, + npmjs.get_publication_datetime, 'return parse_timestamp(_package_metadata(package).get("time", {}).get(version))', 'return parse_timestamp(_package_metadata(package).get("time", {})[version])', "a version the registry dates nowhere in its time map ends the run with a traceback", @@ -193,7 +193,7 @@ def create_responses( @kills( Mutation( - npmjs, + npmjs.get_changes, " return changes_from_release(repository.owner, repository.name, package, version) " "or changes_from_changelog_file(\n" " repository.owner, repository.name, version, repository.directory\n )", @@ -217,7 +217,7 @@ def test_no_changelog_file_when_a_release_matches(self, mock_get: Mock): @kills( Mutation( - npmjs, + npmjs.get_changes, " repository.owner, repository.name, version, repository.directory", ' repository.owner, repository.name, version, ""', "the changelog file of a package a monorepo builds from a directory is looked for in the root", diff --git a/tests/update_time/sources/test_oci.py b/tests/update_time/sources/test_oci.py index 2886f33..b38fbac 100644 --- a/tests/update_time/sources/test_oci.py +++ b/tests/update_time/sources/test_oci.py @@ -103,7 +103,7 @@ def test_an_excluded_reference_keeps_its_version_where_another_resolves(self): @kills( Mutation( - oci, + oci.tag_getter_excluding, ' as_written = f"{pinned.name}{tag_of(pinned.version)}"\n', ' as_written = f"{pinned.name}:{pinned.version}"\n', "a reference naming no tag is matched with an empty tag, so it is looked up on a registry", @@ -182,7 +182,7 @@ def test_up_to_date(self): @kills( Mutation( - oci, + oci.Tag.suffix_label, " return (self._suffix_tag.prefix, self._suffix_tag.suffix)", ' return (self._suffix_tag.prefix, "")', "a suffix's remainder is dropped from its label, so a slim reference drifts onto the fat image", @@ -302,7 +302,7 @@ def test_dated_snapshot_tag_is_no_candidate_for_a_release(self): @kills( Mutation( - oci, + oci.Tag._is_dated_snapshot, " try:\n date.fromisoformat(str(release[0]))\n except ValueError:\n" " return False\n return True", " return True", @@ -382,7 +382,7 @@ def test_newest_release_ignores_labels(self): @kills( Mutation( - oci, + oci._eligible_tag, " if latest is None or not latest.is_eligible(cooldown_days):", " if latest is None or not (latest.is_eligible(cooldown_days) or latest._is_dated_snapshot):", "a dated snapshot is adopted however freshly it was pushed", @@ -591,7 +591,7 @@ def test_alias_on_a_page_after_one_holding_other_digests(self): @kills( Mutation( - docker_hub, + docker_hub.tag_digests, " return digests, not url", " return digests, True", "a tag the listing was cut short before reaching is reported as one the registry does not list", @@ -608,7 +608,7 @@ def test_tag_listed_below_the_page_cap(self): @kills( Mutation( - oci, + oci._unpinned_floating_tag, " served = reason not in (FloatingPin.NOT_LISTED, FloatingPin.NO_MANIFEST)", " served = reason not in (FloatingPin.NOT_LISTED, FloatingPin.NO_MANIFEST, " "FloatingPin.NOT_AMONG_EXAMINED)", @@ -685,13 +685,13 @@ def test_newest_release_for_a_floating_tag(self): @kills( Mutation( - oci, + oci._resolved_tag, " return DependencyVersion(version=current.name, sha=digest, served=bool(digest))", " return DependencyVersion(version=current.name, served=bool(digest))", "a tag naming neither a version nor a channel is left without the digest that would pin it", ), Mutation( - oci, + oci._resolved_tag, " return DependencyVersion(version=current.name, sha=digest, served=bool(digest))", " return DependencyVersion(version=current.name, sha=digest, served=False)", "a tag naming neither a version nor a channel is dated by no release, whatever the registry serves", @@ -712,7 +712,7 @@ def test_tag_naming_neither_a_version_nor_a_channel(self): @kills( Mutation( - oci, + oci._resolved_tag, " return DependencyVersion(version=current.name, sha=digest, served=bool(digest))", " return DependencyVersion(version=current.name, sha=digest)", "a tag the registry serves no manifest for is dated by the image's newest release", @@ -750,7 +750,7 @@ def test_listing_read_once_for_two_floating_tags(self): @kills( Mutation( - oci, + oci._unpinned_floating_tag, " served = reason not in (FloatingPin.NOT_LISTED, FloatingPin.NO_MANIFEST)", " served = reason is not FloatingPin.NO_MANIFEST", "a floating tag the registry does not list is dated by the image's newest release", @@ -775,7 +775,7 @@ def test_listing_unavailable(self): @kills( Mutation( - oci, + oci._walked_floating_tag, """ for candidate in ordered[:_MAX_FLOATING_TAG_PROBES]: if _manifest_digest(image, candidate.name) == digest: return DependencyVersion(version=candidate.name, sha=digest, floating=FloatingPin.RESOLVED)""", @@ -801,7 +801,7 @@ def test_walk_stops_at_the_first_tag_serving_the_digest(self): @kills( Mutation( - oci, + oci._alias_rank, " carried = sum(_carries(alias, label) for label in labels)", " carried = 0", "the walk ignores the floating tag's words, so it lands on the shorter alias it ties with", diff --git a/tests/update_time/sources/test_osv.py b/tests/update_time/sources/test_osv.py index 65bb6d3..d38edfd 100644 --- a/tests/update_time/sources/test_osv.py +++ b/tests/update_time/sources/test_osv.py @@ -91,14 +91,14 @@ def test_advisory_without_a_severity(self, mock_post: Mock): @kills( Mutation( - osv, + osv._vulnerability, 'frozenset(record.get("aliases") or []),', 'frozenset(record.get("aliases", [])),', "an advisory whose aliases OSV reports as null ends the run with a traceback", raises="TypeError: 'NoneType' object is not iterable", ), Mutation( - osv, + osv._banded_risk_level, 'for severity in record.get("severity") or []}', 'for severity in record.get("severity", [])}', "an advisory whose severity OSV reports as null ends the run with a traceback", diff --git a/tests/update_time/sources/test_pypi.py b/tests/update_time/sources/test_pypi.py index 59102e6..0c7c7df 100644 --- a/tests/update_time/sources/test_pypi.py +++ b/tests/update_time/sources/test_pypi.py @@ -69,16 +69,16 @@ def get_latest_version( # The mutations of how a null the PyPI metadata reports for the project URLs is read. The tests of the updater that # rewrites the pins kill them too, so they are named here rather than spelled out in each registration. NULL_PROJECT_URLS_READ_AS_A_DICT = Mutation( - pypi, + pypi.get_changes, 'urls = info.get("project_urls") or {}', 'urls = info.get("project_urls", {})', "the project URLs PyPI reports as null are read as a dictionary, which ends the run with a traceback", raises="AttributeError: 'NoneType' object has no attribute 'items'", ) A_RELEASE_WITHOUT_PROJECT_URLS_SKIPPED = Mutation( - pypi, - " if metadata is None:\n return None", - ' if metadata is None or metadata["info"].get("project_urls", {}) is None:\n return None', + pypi._eligible_release, + " if metadata is None:", + ' if metadata is None or metadata["info"].get("project_urls", {}) is None:', "a release whose project URLs PyPI reports as null is skipped rather than adopted", ) @@ -214,7 +214,7 @@ def test_changelog_url_found(self, mock_get: Mock): @kills( Mutation( - pypi, + pypi._changelog_from_url, " return Changes(changes, markdown=is_markdown_content_type(content_type) or is_markdown_file(url))", " return Changes(changes, markdown=is_markdown_file(url))", "the content type a changelog URL is served with is passed over, so its Markdown is shown raw", @@ -235,13 +235,13 @@ def test_the_content_type_says_the_changelog_url_is_markdown(self, mock_get: Moc @kills( Mutation( - pypi, + pypi._changelog_from_url, " return Changes(changes, markdown=is_markdown_content_type(content_type) or is_markdown_file(url))", " return Changes(changes, markdown=True)", "a changelog URL's extension is passed over, so a reStructuredText changelog is read as Markdown", ), Mutation( - changelog, + changelog.is_markdown_file, " return urlparse(name).path.lower().endswith(MARKDOWN_EXTENSION)", " return name.lower().endswith(MARKDOWN_EXTENSION)", "a query or a fragment after a URL's extension hides it, so its Markdown is shown raw", @@ -268,7 +268,7 @@ def test_the_changelog_url_extension_says_whether_the_changes_are_markdown(self, @kills( Mutation( - pypi, + pypi._changelog_from_url, 'changelog_response.headers.get("Content-Type", "")', 'changelog_response.headers["Content-Type"]', "a changelog URL answered without a content type ends the run with a traceback", @@ -346,18 +346,6 @@ def test_source_url_is_read_before_the_homepage(self, mock_get: Mock): self.assertEqual(get_changes("typing-extensions", "1.1"), changelog) self.assert_releases_requested(mock_get, "python/typing_extensions") - def test_sponsors_project_url_is_not_a_repository(self, mock_get: Mock): - """Test that a GitHub sponsors URL is not asked for releases, and that the later heuristics still run.""" - changelog = "1.1\n- Fixed ...\n- Added ..." - project_urls = {"Funding": "https://github.com/sponsors/webknjaz"} - self.create_mock_response( - mock_get, - {"info": {"description": f"Package description\n{changelog}\n", "project_urls": project_urls}}, - [], - ) - self.assertEqual(get_changes("frozenlist", "1.1"), changelog) - self.assert_releases_requested(mock_get) - @kills(NULL_PROJECT_URLS_READ_AS_A_DICT) def test_changelog_in_description(self, mock_get: Mock): """Test that the description's changelog is returned when PyPI omits the project URLs or reports them null.""" @@ -370,14 +358,14 @@ def test_changelog_in_description(self, mock_get: Mock): @kills( Mutation( - pypi, + pypi._changelog_from_description, " get_version_changes_from_changelog(description, version), " "markdown=is_markdown_content_type(content_type)", " get_version_changes_from_changelog(description, version), markdown=True", "the content type is passed over, so a reStructuredText description is read as Markdown", ), Mutation( - changelog, + changelog.is_markdown_content_type, ' return content_type.partition(";")[0].strip().lower() == _MARKDOWN_CONTENT_TYPE', " return content_type.strip().lower() == _MARKDOWN_CONTENT_TYPE", "a content type carrying parameters matches nothing, so a Markdown description is read as text", @@ -405,7 +393,7 @@ def test_github_url_in_description_that_has_no_changelog(self, mock_get: Mock): self.assertEqual(get_changes("pluggy", "1.1"), "") _FIRST_URL_ONLY = Mutation( - pypi, + pypi._changelog_from_github_url_in_description, " for match in _GITHUB_URL_RE.finditer(description):", " for match in list(_GITHUB_URL_RE.finditer(description))[:1]:", "a package whose description links another project above its own repository reports no changes", @@ -421,14 +409,14 @@ def test_another_projects_url_in_description_is_passed_over(self, mock_get: Mock self.assert_releases_requested(mock_get, "python-attrs/attrs") _NAMES_COMPARED_AS_SPELLED = Mutation( - pypi, + pypi._names_the_package, "return normalized_python_name(repository) == normalized_python_name(package)", "return repository == package", "a package whose repository spells its name with another separator, or in another case, reports no changes", ) _NAME_MATCHED_AS_A_SUBSTRING = Mutation( - pypi, + pypi._names_the_package, "return normalized_python_name(repository) == normalized_python_name(package)", "return normalized_python_name(package) in normalized_python_name(repository)", "a repository whose name merely contains the package's is read as the package's own", @@ -453,24 +441,8 @@ def test_which_repository_names_match_the_package_name(self, mock_get: Mock): self.assertEqual(get_changes(package, "1.1"), changelog if matches else "") self.assert_releases_requested(mock_get, *asked) - _SPONSORS_URL_IS_A_REPOSITORY = Mutation( - pypi, - " _owner, repository = _github_repository(url)", - " _owner, repository = github_owner_and_repository(url)", - "a sponsors page carrying the package's name is read as its repository, so the package reports no changes", - ) - - @kills(_SPONSORS_URL_IS_A_REPOSITORY) - def test_sponsors_url_in_description_is_not_a_repository(self, mock_get: Mock): - """Test that a sponsors URL naming the package is passed over for the repository linked below it.""" - changelog = "1.1\n- Fixed ...\n- Added ..." - description = "Sponsor https://github.com/sponsors/tqdm\nSource https://github.com/tqdm/tqdm\n" - self.create_description_responses(mock_get, description, changelog) - self.assertEqual(get_changes("tqdm", "1.1"), changelog) - self.assert_releases_requested(mock_get, "tqdm/tqdm") - _DOCUMENTATION_READ_FIRST = Mutation( - github, + github.changes_from_changelog_file, " return _changes_from_files(root, version) or " "_changes_from_documentation(owner, repository, root, version)", " return _changes_from_documentation(owner, repository, root, version) or " @@ -508,7 +480,7 @@ def test_changelog_file_in_a_documentation_directory(self, mock_get: Mock): @kills( Mutation( - github, + github._changes_from_tree, " return Changes(changes, markdown=is_markdown_file(name))", " return Changes(changes, markdown=False)", "a Markdown changelog below a documentation directory is read as text, so its markup is shown raw", @@ -544,7 +516,7 @@ def test_root_directory_other_than_documentation(self, mock_get: Mock): self.assertNotIn(tree_url("tests"), requested_urls(mock_get)) _UNFILTERED_TREE = Mutation( - github, + github._list_tree, ' return tuple(entry["path"] for entry in tree if entry["type"] == "blob")', ' return tuple(entry["path"] for entry in tree)', "a directory named like a changelog in a documentation tree costs a request for the file it has none of", @@ -582,7 +554,7 @@ def test_tree_listing_unreachable(self, mock_get: Mock): self.assert_could_not_fetch_logged(url=doc_tree_url) _UNGUARDED_URL = Mutation( - github, + github._changes_from_files, ' if _is_changelog_file(entry["name"]) and url and (changes := ', ' if _is_changelog_file(entry["name"]) and (changes := ', "a directory named like a changelog, such as pip's `news`, costs a request for the file it has none of", @@ -617,7 +589,7 @@ def test_release_is_read_before_the_repository_root(self, mock_get: Mock): self.assert_root_listed(mock_get) _ROOT_FIRST = Mutation( - pypi, + pypi.get_changes, " if changelog := _changelog_from_description(info, package, version):", " for url in repository_urls:\n" " if changelog := _changelog_from_repository_root(url, version):\n" @@ -641,7 +613,7 @@ def test_description_is_read_before_the_repository_root(self, mock_get: Mock): self.assert_root_listed(mock_get) _URL_ENDS_THE_SEARCH = Mutation( - pypi, + pypi.get_changes, " if changelog := _changelog_from_description(info, package, version):", " changelog = _changelog_from_description(info, package, version)\n" ' if changelog or _GITHUB_URL_RE.search(info["description"]):', @@ -721,7 +693,7 @@ def test_root_listing_is_fetched_once_per_repository(self, mock_get: Mock): self.assert_root_listed(mock_get, "googleapis/google-cloud-python") _FIRST_FILE_ONLY = Mutation( - github, + github._changes_from_files, "and url and (changes := _changes_from_changelog_url(url, version)):", "and url and (changes := _changes_from_changelog_url(url, version)) is not None:", "a root whose first changelog file names no version reports no changes, though another file names it", @@ -745,7 +717,7 @@ def test_release_metadata_unreachable(self, mock_get: Mock): @kills( Mutation( - pypi, + pypi._changelog_from_url, "changelog_response = fetch(github_to_raw(url), _LOG, require_ok=False)", "changelog_response = fetch(github_to_raw(url), _LOG)", "a changelog URL the source does not serve warns the reader about a fetch they cannot fix", @@ -772,7 +744,7 @@ def test_changelog_url_not_served(self, mock_get: Mock): @kills( Mutation( - pypi, + pypi._changelog_from_url, " if changelog_response is None:\n return NO_CHANGES\n if not changelog_response.ok:", " if not changelog_response.ok:", "a changelog URL whose request fails ends the run with a traceback", @@ -842,20 +814,20 @@ class ArchivalTest(LoggingTestCase): @kills( Mutation( - pypi, + pypi._archival, ' return Archival(archived=archived, reason=project_status.get("reason") or "")', " return Archival(archived=archived)", "the reason published beside the status is dropped, so an archived project reports none", ), Mutation( - pypi, + pypi._archival, ' project_status = _project_metadata(package).get("project-status") or {}', ' project_status = _project_metadata(package).get("project-status", {})', "a project status PyPI serves as null ends the run with a traceback rather than reading as active", raises="AttributeError: 'NoneType' object has no attribute 'get'", ), Mutation( - pypi, + pypi._archival, ' return Archival(archived=archived, reason=project_status.get("reason") or "")', ' return Archival(archived=archived, reason=project_status.get("reason", ""))', "a reason PyPI serves as null is carried as None in the field that holds the words to quote", @@ -948,7 +920,7 @@ def test_highest_version(self, mock_get: Mock): @kills( Mutation( - pypi, + pypi.get_latest_version, " return replace(latest, project=project(package, check_archival=check_archival))", " _p = project(package, check_archival=check_archival)\n" " return replace(latest, project=Project(" diff --git a/tests/update_time/updaters/test_update_circle_ci_config.py b/tests/update_time/updaters/test_update_circle_ci_config.py index 33e9c01..1fdbf0c 100644 --- a/tests/update_time/updaters/test_update_circle_ci_config.py +++ b/tests/update_time/updaters/test_update_circle_ci_config.py @@ -66,7 +66,7 @@ def test_pin_tagless_image(self): @kills( Mutation( - circle_ci, + circle_ci._machine_images, " images.add(image)\n", ' if ":" in image:\n images.add(image)\n', "a machine-executor image naming no tag is not collected, so it is looked up on a registry", @@ -83,7 +83,7 @@ def test_machine_image_without_a_tag_is_skipped(self): @kills( Mutation( - circle_ci, + circle_ci.update_circle_ci_config, " get_new_version_for=tag_getter_excluding_each(" "_machine_images, AccountedFor.MACHINE_EXECUTOR_IMAGE),\n", " get_new_version_for=tag_getter_excluding_each(_machine_images, AccountedFor.BUILT_IMAGE),\n", diff --git a/tests/update_time/updaters/test_update_dockerfile_base_image.py b/tests/update_time/updaters/test_update_dockerfile_base_image.py index c6f8ca3..a4cc2a2 100644 --- a/tests/update_time/updaters/test_update_dockerfile_base_image.py +++ b/tests/update_time/updaters/test_update_dockerfile_base_image.py @@ -57,7 +57,7 @@ def test_pin_tagless_base_image(self): @kills( Mutation( - update_dockerfile_base_image, + update_dockerfile_base_image._update_dockerfile, " if pinned.name == _SCRATCH:\n", " if pinned.name == _SCRATCH.upper():\n", "a `FROM scratch` is resolved as though a registry served the empty base", @@ -77,7 +77,7 @@ def test_scratch_base_image_is_left_alone_and_not_queried(self): @kills( Mutation( - update_dockerfile_base_image, + update_dockerfile_base_image._update_dockerfile, " return AccountedFor.BUILD_STAGE if pinned.name.lower() in stages else None\n", " return AccountedFor.BUILD_STAGE if pinned.name.lower() in frozenset() else None\n", "a `FROM` naming a build stage is resolved as though a registry served an image of that name", @@ -121,7 +121,7 @@ def test_image_name_from_a_variable_is_left_alone_and_not_queried(self): @kills( Mutation( - log, + log.Logger.keeping_floating_tag, " fields = self._tagged_fields(reference, reference.current_version)\n" " self._log(self._MESSAGE_KEEPING_FLOATING_TAG, **fields, resolved=release.version, " "sha=release.sha, cause=cause)\n", @@ -210,7 +210,7 @@ def test_from_inside_a_comment_is_left_alone(self): @kills( Mutation( - update_dockerfile_base_image, + update_dockerfile_base_image._stage_names, ' return frozenset(match.group("stage").lower() for match in _STAGE_NAME_RE.finditer' "(dockerfile.read_text()))", ' return frozenset(match.group("stage") for match in _STAGE_NAME_RE.finditer(dockerfile.read_text()))', diff --git a/tests/update_time/updaters/test_update_github_action.py b/tests/update_time/updaters/test_update_github_action.py index d17ebac..c8c21fc 100644 --- a/tests/update_time/updaters/test_update_github_action.py +++ b/tests/update_time/updaters/test_update_github_action.py @@ -415,7 +415,7 @@ def test_a_reference_naming_no_repository_is_passed_over(self, mock_glob: Mock): @kills( Mutation( - references_github, + references_github._latest_pin, " warn_about_directives_the_source_cannot_apply(marker, get_latest_version, reference, log)", "", "a reference naming no version has its marker passed over, so a directive holding nothing back is " diff --git a/tests/update_time/updaters/test_update_manifest_images.py b/tests/update_time/updaters/test_update_manifest_images.py index 0ea4984..8834d79 100644 --- a/tests/update_time/updaters/test_update_manifest_images.py +++ b/tests/update_time/updaters/test_update_manifest_images.py @@ -78,7 +78,7 @@ def test_image_whose_tag_the_registry_does_not_serve_is_not_stale(self): @kills( Mutation( - manifest_images, + manifest_images._built_images, " for service in services.values()\n", " for service in list(services.values())[:1]\n", "only the first service is read, so an image a later service builds is looked up on a registry", @@ -111,7 +111,7 @@ def test_every_reference_to_a_built_image_is_left_alone(self): @kills( Mutation( - oci, + oci.tag_getter_excluding, ' without_digests = {image.partition("@")[0] for image in images}\n', " without_digests = set(images)\n", "a digest the file records beside a built image's tag survives into the match, so the image is looked up", @@ -131,7 +131,7 @@ def test_a_built_image_pinned_to_a_digest_is_left_alone(self): @kills( Mutation( - manifest_images, + manifest_images._built_images, ' if isinstance(service, dict) and "build" in service and isinstance(service.get("image"), str)\n', ' if isinstance(service, dict) and "build" in service\n', "a service that builds an image without tagging one aborts the run", @@ -158,21 +158,21 @@ def test_image_the_file_does_not_build_is_updated(self): @kills( Mutation( - manifest_images, + manifest_images._built_images, ' services = document.get("services") if isinstance(document, dict) else None\n', ' services = document.get("services")\n', "a Compose file that is not a mapping is read as one, ending the run", raises="AttributeError: 'list' object has no attribute 'get'", ), Mutation( - manifest_images, + manifest_images._built_images, " if not isinstance(services, dict):\n", " if services is None:\n", "a `services:` that is not a mapping is read as one, ending the run", raises="AttributeError: 'list' object has no attribute 'values'", ), Mutation( - manifest_images, + manifest_images._built_images, ' if isinstance(service, dict) and "build" in service and isinstance(service.get("image"), str)\n', ' if "build" in service and isinstance(service.get("image"), str)\n', "a service declared with no body is read as a mapping, ending the run", @@ -197,7 +197,7 @@ def test_a_malformed_compose_file_is_updated(self): @kills( Mutation( - oci, + oci.tag_getter_excluding, ' as_written = f"{pinned.name}{tag_of(pinned.version)}"\n' " return reason if as_written in without_digests else None\n", ' return reason if pinned.name in {name.split(":", maxsplit=1)[0] for name in without_digests}' @@ -217,7 +217,7 @@ def test_other_tag_of_a_built_repository_is_updated(self): @kills( Mutation( - references_file, + references_file.update_yaml_files, " update_file(path, regexp, get_new_version=get_new_version_for(document), logger=logger)\n", ' getter = getter if "getter" in dir() else get_new_version_for(document)\n' " update_file(path, regexp, get_new_version=getter, logger=logger)\n", @@ -237,7 +237,7 @@ def test_a_file_builds_an_image_for_itself_only(self): @kills( Mutation( - references_resolve, + references_resolve.floating_pin_redundancy, " if not asked:\n return Reason.NO_REGISTRY_ASKED\n if floats is False:\n", " if floats is False:\n", "a reference no registry is asked about is reported as one whose pin does not float, which it may do", diff --git a/tests/update_time/updaters/test_update_node_engine.py b/tests/update_time/updaters/test_update_node_engine.py index d906e70..bf0c33d 100644 --- a/tests/update_time/updaters/test_update_node_engine.py +++ b/tests/update_time/updaters/test_update_node_engine.py @@ -89,7 +89,7 @@ def test_update(self, mock_glob: Mock): @kills( Mutation( - rewrite, + rewrite._reference_match, """ if not reference_marker.reference_location.is_on_the_same_line_as(line.location): return None return pattern.search(line.text, reference_marker.reference_location.column)""", @@ -107,7 +107,7 @@ def test_a_node_version_outside_the_engines_section_is_left_as_it_is(self, mock_ @kills( Mutation( - rewrite, + rewrite._reference_match, " return pattern.search(line.text, reference_marker.reference_location.column)", " return pattern.search(line.text)", "the named line is read from its start, so the entry declared before the engine is taken for it", @@ -134,7 +134,7 @@ def test_non_numeric_node_base_image(self, mock_glob: Mock): @kills( Mutation( - update_node_engine, + update_node_engine._node_base_image_version, ' return version if is_valid(version) else ""', " return version", "a tag's digits are adopted as a version even when they do not form one, ending the run", @@ -155,7 +155,7 @@ def test_unreadable_node_base_image_version(self, mock_glob: Mock): @kills( Mutation( - update_node_engine, + update_node_engine._has_node_engine, " return isinstance(engines, dict) and _NODE in engines", " return engines is not None and _NODE in engines", "a section that is a string or a list is read as one naming the engine, so the file is opened for it", @@ -178,7 +178,7 @@ def test_no_node_engine(self, mock_glob: Mock): @kills( Mutation( - update_node_engine, + update_node_engine._update_node_engine, """ if engine_marker.reference_location.line_number is None: _LOG.no_entry(_NODE, package_json.path) return @@ -274,7 +274,7 @@ def test_the_engine_is_updated_when_the_marker_field_is_declared_above_it(self, @kills( Mutation( - update_node_engine, + update_node_engine._update_node_engine, "logger=_LOG, reference_marker=engine_marker", "logger=_LOG", "the engine's entry and marker are read but never handed to the gate, where it follows Docker Hub", @@ -337,7 +337,7 @@ def test_invalid_item_in_the_update_time_field_leaves_the_engine_unchanged(self, @kills( Mutation( - package_json, + package_json._marker, """ if not isinstance(field, dict): return _unreadable_field(section, name)""", """ if not isinstance(field, dict): @@ -345,7 +345,7 @@ def test_invalid_item_in_the_update_time_field_leaves_the_engine_unchanged(self, "a field that is not the object it should be reads as naming no marker", ), Mutation( - package_json, + package_json._marker, """ if not isinstance(references, dict): return _unreadable_field(section, name)""", """ if not isinstance(references, dict): @@ -353,7 +353,7 @@ def test_invalid_item_in_the_update_time_field_leaves_the_engine_unchanged(self, "a section that is not the object it should be reads as naming no marker", ), Mutation( - package_json, + package_json._marker, " return parse_directives(directives) if isinstance(directives, str)" " else _unreadable_field(section, name)", " return parse_directives(directives)", @@ -397,7 +397,7 @@ def test_an_engine_ahead_of_the_base_image_follows_it_down(self, mock_glob: Mock @kills( Mutation( - resolve, + resolve.latest_version, " if not downgrades(get_new_version, reference.pinned):\n" " log.warn_if_redundant_bound(reference, marker)", " log.warn_if_redundant_bound(reference, marker)", diff --git a/tests/update_time/updaters/test_update_pom_xml.py b/tests/update_time/updaters/test_update_pom_xml.py index 216fbc3..e26f09e 100644 --- a/tests/update_time/updaters/test_update_pom_xml.py +++ b/tests/update_time/updaters/test_update_pom_xml.py @@ -9,7 +9,7 @@ from unittest.mock import Mock, patch from update_time.domain.cooldown import COOLDOWN -from update_time.domain.dependency import Archival, ArchivedSubject, Project, Release +from update_time.domain.dependency import NO_CHANGES, Archival, ArchivedSubject, Changes, Project, Release from update_time.io.log import Logger from update_time.manifests import pom_xml as pom_xml_module from update_time.package_managers import maven as maven_module @@ -159,6 +159,7 @@ def _maven_command(executable: str = "mvn", rules: Path | None = None) -> Comman @no_vulnerabilities @patch.object(maven_central_module, "project", Mock(return_value=Project())) +@patch.object(maven_central_module, "get_changes", Mock(return_value=NO_CHANGES)) @patch.object(maven_module, "versions_within_cooldown", Mock(return_value=())) @patch_pathlib_path("rglob", cwd=Path("/"), exists=False) @patch("subprocess.run") @@ -273,30 +274,41 @@ def test_osv_is_asked_about_the_version_the_run_lands_on(self, mock_run: Mock, m @kills( Mutation( pom_xml_module.fully_resolved, - " and _is_resolved(reference.current_version)", - "", + "(group_id, artifact_id, pinned.version)", + "(group_id, artifact_id)", "OSV is asked about a version holding an unresolved property, which it matches nothing to", ), Mutation( pom_xml_module.artefact_references, "_is_resolved(reference.dependency)", - "fully_resolved(reference)", + "fully_resolved(reference.pinned)", "staleness judges the version too, so a dependency its parent versions goes unchecked for years", ), + Mutation( + pom_xml_module.fully_resolved, + "part and _is_resolved(part)", + "_is_resolved(part)", + "OSV is asked about a dependency at an empty version, which it matches nothing to", + ), ) - def test_a_version_naming_a_property_is_asked_about_for_staleness_alone(self, mock_run: Mock, mock_glob: Mock): - """Test that a dependency whose `` names a property the pom lacks reaches Maven Central, not OSV.""" + def test_a_version_naming_a_property_or_left_empty_is_asked_about_for_staleness_alone( + self, mock_run: Mock, mock_glob: Mock + ): + """Test that a dependency whose `` names a property or nothing reaches Maven Central, not OSV.""" # A pom can spell a dependency's version as a property its parent declares, which this pom does not hold. - inherited = _dependency("org.springframework", "spring-core", "${spring.version}") - self.find_pom(mock_run, mock_glob, _pom(_guava("33.0.0-jre"), inherited)) - mock_project = Mock(return_value=Project()) - with osv() as mock_post, patch.object(maven_central_module, "project", mock_project): - update_pom_xmls() - # Guava is asked about, so the dependency beside it going unasked says something about its version. - assert_osv_asked_about(mock_post, ("com.google.guava:guava", "33.0.0-jre"), ecosystem="Maven") - # Staleness judges the coordinates alone, so the dependency OSV skips still reaches Maven Central. - asked = ["com.google.guava:guava", "org.springframework:spring-core"] - self.assertEqual([call.args[0] for call in mock_project.call_args_list], asked) + for case, version in {"property": "${spring.version}", "empty": ""}.items(): + with self.subTest(case=case): + self.clear_caches() + unversioned = _dependency("org.springframework", "spring-core", version) + self.find_pom(mock_run, mock_glob, _pom(_guava("33.0.0-jre"), unversioned)) + mock_project = Mock(return_value=Project()) + with osv() as mock_post, patch.object(maven_central_module, "project", mock_project): + update_pom_xmls() + # Guava is asked about, so the dependency beside it going unasked says something about its version. + assert_osv_asked_about(mock_post, ("com.google.guava:guava", "33.0.0-jre"), ecosystem="Maven") + # Staleness judges the coordinates alone, so the dependency OSV skips still reaches Maven Central. + asked = ["com.google.guava:guava", "org.springframework:spring-core"] + self.assertEqual([call.args[0] for call in mock_project.call_args_list], asked) @kills( Mutation( @@ -430,27 +442,37 @@ def test_each_artefact_gets_a_rule_naming_its_own_held_back_versions(self, mock_ ), Mutation( pom_xml_module.fully_resolved, - "_is_resolved(reference.dependency) and ", - "", + "(group_id, artifact_id, pinned.version)", + "(pinned.version,)", "OSV is asked about coordinates holding an unresolved property, which it matches nothing to", ), + Mutation( + update_pom_xml_module._report_new_versions, + " resolved = pom_xml_format.fully_resolved(new.pinned)", + " resolved = True", + "Maven Central is asked for the changes of coordinates holding an unresolved property", + ), ) def test_coordinates_naming_a_property_are_not_asked_about(self, mock_run: Mock, mock_glob: Mock): """Test that every check passes over a dependency whose coordinates name a property the pom lacks.""" # A pom can spell a dependency's group as a property its parent declares, which this pom does not hold. - inherited = _dependency("${spring.group}", "spring-core", "6.1.0") - self.find_pom(mock_run, mock_glob, _pom(_guava("33.0.0-jre"), inherited)) + inherited = _dependency("${spring.group}", "spring-core", "${spring.version}") + before = _pom(_guava("33.0.0-jre"), inherited, properties=_properties({"spring.version": "6.1.0"})) + after = _pom(_guava("33.7.1-jre"), inherited, properties=_properties({"spring.version": "7.1.0"})) + self.find_rewritten_pom(mock_run, mock_glob, before, after) mock_project = Mock(return_value=Project()) with ( self.asked_about() as artefacts, osv() as mock_post, patch.object(maven_central_module, "project", mock_project), + patch.object(maven_central_module, "get_changes", Mock(return_value=NO_CHANGES)) as mock_changes, ): update_pom_xmls() # Guava reaches every check, so the dependency beside it reaching none says something about its coordinates. self.assertEqual(artefacts, ["com.google.guava:guava"]) - assert_osv_asked_about(mock_post, ("com.google.guava:guava", "33.0.0-jre"), ecosystem="Maven") + assert_osv_asked_about(mock_post, ("com.google.guava:guava", "33.7.1-jre"), ecosystem="Maven") self.assertEqual([call.args[0] for call in mock_project.call_args_list], ["com.google.guava:guava"]) + self.assertEqual([call.args[0] for call in mock_changes.call_args_list], ["com.google.guava:guava"]) @kills( Mutation( @@ -536,19 +558,37 @@ def test_a_failed_maven_run_is_reported_with_its_output(self, mock_run: Mock, mo @kills( Mutation( update_pom_xml_module._report_new_versions, - "_LOG.new_version(updated, DependencyVersion(new.current_version))", - "_LOG.new_version(updated, DependencyVersion(new.current_version))\n return", + "_LOG.new_version(updated, DependencyVersion(new.current_version, changes))", + "_LOG.new_version(updated, DependencyVersion(new.current_version, changes))\n return", "only the first dependency Maven moved is reported, and the rest of the pom's are lost", - ) + ), + Mutation( + update_pom_xml_module._report_new_versions, + "get_changes(new.dependency, new.current_version)", + "get_changes(new.dependency, old.current_version)", + "a dependency Maven moved is reported with the changes of the version it moved away from", + ), ) - def test_each_rewritten_version_is_reported_at_its_own_line(self, mock_run: Mock, mock_glob: Mock): - """Test that two dependencies Maven rewrote are both reported, each at its own version's line.""" + def test_each_rewritten_version_is_reported_at_its_own_line_with_its_changes(self, mock_run: Mock, mock_glob: Mock): + """Test that two dependencies Maven rewrote are both reported, each at its own line with its new changes.""" before = _pom(_guava("33.0.0-jre"), _dependency("org.springframework", "spring-core", "6.1.0")) after = _pom(_guava("33.7.1-jre"), _dependency("org.springframework", "spring-core", "7.1.0")) pom = self.find_rewritten_pom(mock_run, mock_glob, before, after) - update_pom_xmls() - self.assert_new_version_logged_among_others("com.google.guava:guava", "33.7.1-jre", Location(pom, 6)) - self.assert_new_version_logged_among_others("org.springframework:spring-core", "7.1.0", Location(pom, 11)) + + def changes(artefact: str, version: str) -> Changes: + return Changes(f"Changes in {artefact} {version}", markdown=False) + + with patch.object(maven_central_module, "get_changes", changes): + update_pom_xmls() + self.assert_new_version_logged_among_others_with_changes( + "com.google.guava:guava", "33.7.1-jre", Location(pom, 6), "Changes in com.google.guava:guava 33.7.1-jre" + ) + self.assert_new_version_logged_among_others_with_changes( + "org.springframework:spring-core", + "7.1.0", + Location(pom, 11), + "Changes in org.springframework:spring-core 7.1.0", + ) self.assertEqual(len(self.new_version_records()), 2) @kills( @@ -602,15 +642,24 @@ def test_a_name_two_sections_declare_is_reported_per_declaration(self, mock_run: "old.current_version == new.current_version", "False", "every dependency is reported as moved, whether or not Maven changed its version", - ) + ), + Mutation( + update_pom_xml_module._report_new_versions, + " if old.current_version == new.current_version:\n", + " maven_central.get_changes(new.dependency, new.current_version)\n" + " if old.current_version == new.current_version:\n", + "Update-time asks for the changes of every dependency the pom declares, whether or not Maven moved it", + ), ) - def test_a_dependency_maven_left_alone_is_not_reported(self, mock_run: Mock, mock_glob: Mock): - """Test that a pom Maven changed nothing in gets no new version reported, though Maven ran over it.""" + def test_a_dependency_maven_left_alone_is_neither_reported_nor_asked_about(self, mock_run: Mock, mock_glob: Mock): + """Test that Update-time neither reports a new version nor asks for changes when Maven leaves the pom alone.""" unchanged = _pom(_guava("33.7.1-jre")) self.find_rewritten_pom(mock_run, mock_glob, unchanged, unchanged) - update_pom_xmls() + with patch.object(maven_central_module, "get_changes") as get_changes: + update_pom_xmls() self.assert_maven_ran(mock_run) # The pom was examined, so reporting nothing says something. self.assert_no_new_version_logged() + get_changes.assert_not_called() @kills( Mutation( diff --git a/tests/update_time/updaters/test_update_pyproject_toml.py b/tests/update_time/updaters/test_update_pyproject_toml.py index 6fe3fb6..77ea9a2 100644 --- a/tests/update_time/updaters/test_update_pyproject_toml.py +++ b/tests/update_time/updaters/test_update_pyproject_toml.py @@ -155,7 +155,7 @@ def test_update_of_a_dependency_the_file_declares_nowhere(self, run: Mock, get: @kills( Mutation( - uv, + uv._update_dependencies, "_with_reported_markers(pyproject_toml_format.declared_dependencies(file), log)", "_with_reported_markers(" "pyproject_toml_format.declared_dependencies(file) if lines_with_updates else [], log)", @@ -173,7 +173,7 @@ def test_a_marker_on_a_pin_uv_reports_no_update_for_is_reported(self, run: Mock, @kills( Mutation( - pyproject_toml, + pyproject_toml.Declaration.updatable, " return not (self.pins_a_version and self.marker.ignores(Scope.UPDATE))", " return not self.marker.ignores(Scope.UPDATE)", "an `ignore[update]` holds back a dependency that pins no version, whose version uv resolves whatever " @@ -192,7 +192,7 @@ def test_a_new_version_is_reported_for_a_dependency_that_pins_no_version(self, r @kills( Mutation( - uv, + uv._reported_marker, " lambda: log.ignored(*reported) if declaration.pins_a_version else None,", " lambda: log.ignored(*reported),", "a dependency that pins no version reads as one whose update was held back, though the marker froze no " @@ -211,7 +211,7 @@ def test_no_update_is_reported_as_held_back_for_a_dependency_that_pins_no_versio @kills( Mutation( - uv, + uv._update_dependencies, "if declared else [Location(file.path)]", "or [Location(file.path)]", "a package whose every declaration is held back has its new version reported at the file, and costs a " @@ -231,7 +231,7 @@ def test_a_marker_holds_the_update_back(self, run: Mock, get: Mock, glob: Mock): @kills( Mutation( - uv, + uv._update_dependencies, "if declaration.updatable", "if all(other.updatable for other in declared)", "a marker freezes the name rather than the declaration, so another declaration of that name is frozen too", @@ -247,7 +247,7 @@ def test_a_marker_freezes_one_declaration_of_a_name_while_the_other_updates(self @kills( Mutation( - uv, + uv._with_reported_markers, "replace(declaration, marker=_reported_marker(declaration, log))", "(_reported_marker(declaration, log), declaration)[1]", "the marker a report settles on is discarded, so an unreadable item warns and the pin it may have been " @@ -269,7 +269,7 @@ def test_an_invalid_bracket_item_leaves_the_pin_unchanged(self, run: Mock, get: @kills( Mutation( - uv, + uv._update_dependencies, "if declaration.updatable", "if declaration.updatable and not any(one.marker.invalid_item for one in declared)", "an unreadable item freezes every declaration of its name rather than the one carrying it", @@ -287,14 +287,14 @@ def test_an_invalid_item_freezes_one_declaration_of_a_name_while_the_other_updat @kills( Mutation( - uv, + uv._reported_marker, "log.report_inverted_items(declaration, declaration.marker)", "log.report_inverted_items(declaration, type(declaration.marker)())", "the declaration's own marker never reaches the warning, so an item comparing the wrong way is left " "unreported and reads as understood", ), Mutation( - uv, + uv._with_reported_markers, "[replace(declaration, marker=_reported_marker(declaration, log)) for declaration in declarations]", "[replace(declaration, marker=_reported_marker(declaration, log)) " "if declaration.current_version else declaration for declaration in declarations]", @@ -321,7 +321,7 @@ def test_an_inverted_comparison_is_reported(self, run: Mock, get: Mock, glob: Mo @kills( Mutation( - uv, + uv._reported_marker, "return acted_on", "return acted_on.frozen if any(threshold.inverted_item for threshold in " "(declaration.marker.stale, declaration.marker.cooldown, declaration.marker.vulnerable)) else acted_on", diff --git a/tests/update_time/updaters/test_update_python_version_file.py b/tests/update_time/updaters/test_update_python_version_file.py index 8bbeb3e..9176c4e 100644 --- a/tests/update_time/updaters/test_update_python_version_file.py +++ b/tests/update_time/updaters/test_update_python_version_file.py @@ -63,7 +63,7 @@ def test_update_from_dockerfile(self, mock_glob: Mock): @kills( Mutation( - update_python_version_file, + update_python_version_file._find_python_base_image_version, " for dockerfile in (local_dockerfile, *glob_for(DOCKERFILES)):", " for dockerfile in (local_dockerfile,):", "the base image is looked for beside the version file alone, not anywhere in the repository", diff --git a/tests/update_time/updaters/test_update_requirements_txt.py b/tests/update_time/updaters/test_update_requirements_txt.py index 7de4100..980a64e 100644 --- a/tests/update_time/updaters/test_update_requirements_txt.py +++ b/tests/update_time/updaters/test_update_requirements_txt.py @@ -214,7 +214,7 @@ def test_a_requirement_whose_newest_release_is_recent_is_not_warned(self, mock_r @kills( Mutation( - resolve_module, + resolve_module.report_project, " log.report_archival(resolved, resolved.marker)", " if staleness_threshold(resolved.marker):\n log.report_archival(resolved, resolved.marker)", "the archival check sits behind the staleness gate, so switching staleness off silences archival too", @@ -276,7 +276,7 @@ def test_archived_loose_requirement_warned(self, mock_rglob: Mock, mock_get: Moc @kills( Mutation( - pypi_module, + pypi_module._archival, " if not check_archival:\n return Archival()\n", "", "the source is told not to check for archival but answers anyway, so an archived project is warned " @@ -682,9 +682,8 @@ def test_ignore_vulnerable_advisory_marker_silences_that_advisory(self, mock_rgl @kills( Mutation( - log_module, - " reference.location,\n advisory=vulnerability.advisory,", - " reference.location,\n" + log_module.Logger.ignored_vulnerability, + " advisory=vulnerability.advisory,", " advisory=next(iter(marker.ignored_advisories), vulnerability.advisory),", "the line names the identifier the marker spelled rather than the one OSV answered under", ) @@ -731,7 +730,7 @@ def test_ignore_vulnerable_advisory_marker_still_warns_about_another_advisory( @kills( Mutation( - vulnerability_module, + vulnerability_module._warn_about_vulnerable_versions, " for vulnerability in vulnerabilities:", " for vulnerability in (vulnerabilities[:1] " "if reference.marker.ignores(Scope.VULNERABLE) else vulnerabilities):", @@ -813,7 +812,7 @@ def test_globally_ignored_advisory_silences_the_warning(self, mock_rglob: Mock, @kills( Mutation( - vulnerability_module, + vulnerability_module._report, " logger.globally_ignored_vulnerability(reference, vulnerability, silenced_by)", " logger.globally_ignored_vulnerability(reference, vulnerability, " "frozenset({vulnerability.advisory}))", diff --git a/tests/update_time/updaters/test_uv_pins.py b/tests/update_time/updaters/test_uv_pins.py index 721c289..c572e65 100644 --- a/tests/update_time/updaters/test_uv_pins.py +++ b/tests/update_time/updaters/test_uv_pins.py @@ -93,7 +93,7 @@ def test_stale_dependency_warned(self, get: Mock): @kills( Mutation( - toml_module, + toml_module.parse_document, " except tomlkit.exceptions.TOMLKitError:", " except tomlkit.exceptions.ParseError:", "TOML that tomlkit rejects with the base error aborts the run instead of leaving the file out", @@ -130,14 +130,10 @@ def test_stale_pin_a_marker_silences_not_warned(self, get: Mock): @kills( Mutation( - uv_module, - " for declaration in declarations:\n" - " release = DependencyVersion.unpinned(" - "project(declaration.dependency, check_archival=archival_is_checked()))", + uv_module.pypi_projects, + " for declaration in declarations:", " steered = list(declarations)\n" - " for declaration in [replace(one, marker=steered[0].marker) for one in steered]:\n" - " release = DependencyVersion.unpinned(" - "project(declaration.dependency, check_archival=archival_is_checked()))", + " for declaration in [replace(one, marker=steered[0].marker) for one in steered]:", "one declaration's marker steers every dependency the file declares, rather than the one carrying it", ) ) @@ -174,7 +170,7 @@ def test_an_inverted_item_sets_no_threshold(self, get: Mock): @kills( Mutation( - delegated_module, + delegated_module._checked_references, "project_is_checked(projects, reference.dependency, staleness_threshold(reference.marker))", "project_is_checked(projects, reference.dependency, staleness_threshold(type(reference.marker)()))", "a reference's own threshold does not reach the gate, so a run with both checks off looks it up for none", @@ -258,7 +254,7 @@ def test_a_marker_silences_the_yank_warning(self, get: Mock): @kills( Mutation( - uv_module, + uv_module.pinned_pypi_releases, "SteeredResolvedReference.from_reference(pin, release=DependencyVersion(pin.current_version, yank=yank))", "SteeredResolvedReference.from_reference(" "pin, release=DependencyVersion(pin.current_version, yank=yank), " @@ -424,7 +420,7 @@ def test_archived_dependency_a_marker_silences_not_warned(self, get: Mock): @kills( Mutation( - marker_reference_module, + marker_reference_module.SteeredReference.from_reference, ' resolved.setdefault("marker", reference.marker)', ' resolved.setdefault("marker", reference.marker if reference.current_version ' "else type(reference.marker)())", @@ -446,7 +442,7 @@ def test_a_marker_silences_a_dependency_that_pins_no_version(self, get: Mock): @kills( Mutation( - delegated_module, + delegated_module._asks_its_source, " return not reference.marker.holds_back_source_checks", " return True", "a marker holding back every check PyPI answers still costs a request for the dependency it steers", @@ -497,7 +493,7 @@ def test_archived_pin_warned_at_the_cost_of_one_request(self, get: Mock): @kills( Mutation( - uv_module, + uv_module.pypi_projects, " yield SteeredResolvedReference.from_reference(declaration, release=release)", " if release.project.newest is not None:\n" " yield SteeredResolvedReference.from_reference(declaration, release=release)", @@ -524,7 +520,7 @@ def check_dependencies(self, get: Mock, file: PyprojectToml) -> None: @kills( Mutation( - delegated_module, + delegated_module._scopes_that_need_a_version, " if directive.without_a_version is not None and (written := as_written.directive_for", " if directive.scope is Scope.YANKED and (written := as_written.directive_for", "only the yank scope is reported as needing a version, so a `vulnerable` scope on a dependency that " @@ -547,7 +543,7 @@ def test_a_scope_needing_a_version_is_redundant_without_an_exact_pin(self, get: @kills( Mutation( - delegated_module, + delegated_module._redundant_directives, " yield from _redundant_cooldown_directive(reference)\n", "", "a cooldown of a dependency's own reads as applying to it, though uv takes one per run", @@ -565,9 +561,9 @@ def test_a_cooldown_is_reported_as_applying_per_run(self, get: Mock): @kills( Mutation( - delegated_module, - " if written := as_written.directive_for(directive.scope):\n yield written, reason", - " if written := as_written.directive_for(directive.scope):\n yield written, reason\n" + delegated_module._warning_scopes, + " yield written, reason", + " yield written, reason\n" " if cooldown := as_written.cooldown_directive:\n yield cooldown, reason", "the rule for a dependency no source reports on sweeps the cooldown in with the warnings, so one item " "is reported twice", @@ -599,16 +595,16 @@ def unserved_declarations(directive: str) -> dict[str, str]: @kills( Mutation( - delegated_module, - " as_written = reference.marker.as_written\n if as_written.ignores(Scope.UPDATE)", - " as_written = reference.marker\n if as_written.ignores(Scope.UPDATE)", + delegated_module._redundant_update_directives, + " as_written = reference.marker.as_written", + " as_written = reference.marker", "the update rules read the scopes a bare `ignore` holds back without naming, so they report an " "`ignore[update]` the reader never wrote", ), Mutation( - delegated_module, - " if reference.current_version:\n return\n as_written = reference.marker.as_written", - " if reference.current_version:\n return\n as_written = reference.marker", + delegated_module._scopes_that_need_a_version, + " as_written = reference.marker.as_written", + " as_written = reference.marker", "the scopes rule reads the scopes a bare `ignore` holds back without naming, so it reports scopes the " "reader never wrote", ), @@ -639,7 +635,7 @@ def test_an_ignore_update_freezing_an_unserved_pin_is_not_reported(self, get: Mo @kills( Mutation( - uv_module, + uv_module.no_pypi_release, " return Reason.NO_PYPI_RELEASE if declaration.names_no_release else None", " return None", "a dependency PyPI serves no release for is judged as if it did, so a warning scope on it is reported " @@ -659,7 +655,7 @@ def test_a_scope_is_reported_for_a_dependency_pypi_serves_no_release_for(self, g @kills( Mutation( - delegated_module, + delegated_module._redundant_update_directives, " if bound := as_written.version_bound_directive:", " if as_written.ignores(Scope.UPDATE) and (bound := as_written.version_bound_directive):", "a bound goes on deciding nothing without a word, as it did before uv-resolved dependencies took a marker", @@ -678,7 +674,7 @@ def test_a_bound_is_reported_as_deciding_nothing(self, get: Mock): @kills( Mutation( - delegated_module, + delegated_module._redundant_update_directives, " if bound := as_written.version_bound_directive:", " if not as_written.ignores(Scope.UPDATE) and (bound := as_written.version_bound_directive):", "the `ignore[update]` beside a bound hides it, so the directive that decides nothing goes unreported", @@ -694,7 +690,7 @@ def test_a_bound_beside_a_freeze_is_reported(self, get: Mock): @kills( Mutation( - delegated_module, + delegated_module._redundant_floating_pin_directive, "floating_pin_redundancy(reference.marker, floats=False)", "floating_pin_redundancy(reference.marker, floats=None)", "a Python dependency's pin reads as one that might float, so an `allow[floating-pin]` keeping nothing " @@ -711,7 +707,7 @@ def test_a_floating_pin_directive_is_reported(self, get: Mock): @kills( Mutation( - delegated_module, + delegated_module._redundant_floating_pin_directive, "floating_pin_redundancy(reference.marker, floats=False)", "floating_pin_redundancy(reference.marker.as_written, floats=False)", "the rule reads the marker as written, so a bare `ignore` reads as holding the update back not, and its " @@ -730,7 +726,7 @@ def test_a_floating_pin_directive_is_reported_as_a_held_back_update(self, get: M @kills( Mutation( - delegated_module, + delegated_module.warn_about_redundant_directives, " for reference in chain.from_iterable(declared):\n" " for written, reason in _redundant_directives(reference, no_source_for(reference)):", " references = list(chain.from_iterable(declared))\n" @@ -754,7 +750,7 @@ class UvSourcedDependencyTest(DependencyTomlFileTestCase): @kills( Mutation( - uv_pins_module, + uv_pins_module.warn_about_pins, " served = [uv.pypi_served(declarations) for declarations in declared]", " served = [list(declarations) for declarations in declared]", "the checks are handed every declaration, so PyPI is asked about one it serves no release for", diff --git a/tools/prose-whitelist.txt b/tools/prose-whitelist.txt index eac52a6..2f87885 100644 --- a/tools/prose-whitelist.txt +++ b/tools/prose-whitelist.txt @@ -14,7 +14,6 @@ A URL the mapping doesn't cover raises a KeyError, so a request the test did not A URL whose attribute dictionary declares no `integrity` entry gains one, so the browser verifies the script the CDN serves before running it. A `*.py` file without such a block stays untouched and never invokes uv. A `.python-version` entry and a Node engine both follow the base image in the project's Dockerfile, so the runtime a project develops against and the runtime it ships stay in step. -A `github.com/sponsors/…` URL parses as the repository `sponsors/`, which does not exist, so it is reported as no repository rather than queried. A `node` version another section declares — a Volta pin, a dependency of that name — is neither taken for the engine nor steered by the engine's marker. A `package.json` dependency takes no marker, because npm and pnpm update it rather than Update-time rewriting its lines. A `package.json` dependency takes no marker. @@ -318,7 +317,6 @@ Return the index of the line heading the next version's section, or None when no Return the level the vector's base score bands into, or no level when the vector cannot be scored. Return the marker a risk level bracket item expresses, or None when the item names no risk level. Return the marker an advisory bracket item expresses, or None when the item names no advisory. -Return the owner and repository the URL points at, or empty strings when it points at no repository. Return the public module-level constants that no module other than the one assigning them refers to. Return the public module-level definitions that no module other than the defining one refers to. Return the public names each module yields that no module other than the one defining them refers to. @@ -432,7 +430,6 @@ Test that a non-OK response yields no release, and is reported as a failed fetch Test that a package whose description holds the changes costs no root listing, and reports those. Test that a package whose release answers costs no root listing, and reports the release's changes. Test that a package.json whose `engines` section declares no Node version is skipped. -Test that a pom Maven changed nothing in gets no new version reported, though Maven ran over it. Test that a pom declaring no release raises an error naming the file, rather than one about a lookup. Test that a pom whose XML does not parse declares no property, rather than ending the run. Test that a property Maven advanced is reported for no dependency when no `` names it. @@ -490,7 +487,6 @@ Test that an advisory whose CVSS vector cannot be scored is read at no level, an Test that an anonymous registry token is requested (no basic auth) when no credentials are set. Test that an archived project is warned about although the index lists no release for it. Test that an artefact whose listing dates no version has no pom to read, so nothing follows the listing. -Test that an artefact whose pom declares no `` element leaves GitHub unasked. Test that an entry named like a changelog with no file to fetch is passed over without a request. Test that an excluded reference's versions carry no publication date, so a cooldown measures against none. Test that an inline `# update-time: ignore` comment leaves the action untouched, looking up no version. @@ -841,7 +837,6 @@ a service declared with no body is read as a mapping, ending the run. a service has no body. a snapshot's label is read as no part of its tag, so the pin loses it and crosses to another line. a source is asked about a reference no check needs an answer for, so the run pays for the request. -a sponsors page carrying the package's name is read as its repository, so the package reports no changes. a tag naming neither a version nor a channel is dated by no release, whatever the registry serves. a tag the listing was cut short before reaching is reported as one the registry does not list. a tag the registry serves no manifest for is dated by the image's newest release. diff --git a/tools/readability_check.py b/tools/readability_check.py index 9d020e9..e3eb5d5 100644 --- a/tools/readability_check.py +++ b/tools/readability_check.py @@ -1,5 +1,6 @@ """Report hard-to-read sentences in the prose of the Python and Markdown files under a directory.""" +import argparse import ast import inspect import io @@ -18,7 +19,7 @@ from tools.markdown import lines_without_code_blocks if TYPE_CHECKING: - from collections.abc import Callable, Iterator + from collections.abc import Callable, Iterable, Iterator _INLINE_CODE = re.compile(r"`[^`]*`") @@ -61,12 +62,6 @@ # A parenthesised aside without an aside of its own, so repeating the substitution reaches the nested ones too. _ASIDE = re.compile(r"\([^()]*\)") -# The file listing the sentences this check passes over, and the argument that writes them out to regenerate it. -_WHITELIST = Path("tools/prose-whitelist.txt") -_MAKE_WHITELIST = "--make-whitelist" -_CHECK_WHITELIST = "--check-whitelist" -_INSTALL_DATA = "--install-data" - # Below this many words a ratio says more about a sentence's length than about its density. _RATIO_WORDS = 15 @@ -368,7 +363,7 @@ def _faults(sentence: str, limits: _Limits) -> str: if _subject_is_split(tagged): faults.append("subject split from its verb") if _negates_a_noun_phrase(tagged): - faults.append("negation in a noun phrase") + faults.append("negation in a noun phrase (put the negation on the verb, as in 'does not have a release')") return " and ".join(faults) @@ -395,12 +390,14 @@ def _normalized(sentence: str) -> str: return " ".join(sentence.split()) -def _whitelisted_sentences() -> set[str]: - """Return the sentences the whitelist holds, which this check passes over.""" - return set(_WHITELIST.read_text().splitlines()) if _WHITELIST.exists() else set() +def _whitelisted_sentences(whitelist: Path | None) -> set[str]: + """Return the sentences the whitelist file holds, which this check passes over.""" + if whitelist is None or not whitelist.exists(): + return set() + return set(whitelist.read_text().splitlines()) -type _Flagged = Iterator[tuple[Prose, str, str]] +type _Flagged = Iterable[tuple[Prose, str, str]] def _flagged(paths: list[Path], limits: _Limits) -> _Flagged: @@ -415,19 +412,26 @@ def _flagged(paths: list[Path], limits: _Limits) -> _Flagged: def main() -> int: """Report the sentences that are hard to read, in the files under the paths given or the current directory. - Passing `--make-whitelist` writes the sentences out for the whitelist file to hold, rather than reporting them. - Passing `--check-whitelist` reports the entries the whitelist no longer needs, which only a run over every file - can tell. + Passing `--make-whitelist FILE` rewrites FILE to hold the sentences it holds that the prose still has. + Passing `--whitelist FILE` passes over the sentences FILE holds. Passing `--check-whitelist FILE` does so too, and + also reports the entries FILE no longer needs, which only a run over every file can tell. """ - arguments = sys.argv[1:] - if _INSTALL_DATA in arguments: + parser = argparse.ArgumentParser() + parser.add_argument("paths", nargs="*", type=Path, default=[Path()]) + parser.add_argument("--install-data", action="store_true") + whitelist = parser.add_mutually_exclusive_group() + whitelist.add_argument("--whitelist", type=Path) + whitelist.add_argument("--check-whitelist", type=Path) + whitelist.add_argument("--make-whitelist", type=Path) + arguments = parser.parse_args() + if arguments.install_data: return _install_data() - options = {_MAKE_WHITELIST, _CHECK_WHITELIST} - paths = [Path(start) for start in arguments if start not in options] or [Path()] - flagged = _flagged(paths, _Limits()) - if _MAKE_WHITELIST in arguments: - return _write_whitelist(flagged) - return _report(flagged, checking_whitelist=_CHECK_WHITELIST in arguments) + flagged = _flagged(arguments.paths, _Limits()) + if arguments.make_whitelist: + return _write_whitelist(flagged, arguments.make_whitelist) + if arguments.check_whitelist: + return _check_whitelist(flagged, arguments.check_whitelist) + return _report(flagged, arguments.whitelist) def _install_data() -> int: @@ -440,34 +444,40 @@ def _install_data() -> int: return 0 -def _write_whitelist(flagged: _Flagged) -> int: - """Write the flagged sentences to standard output, sorted and one per line. +def _write_whitelist(flagged: _Flagged, whitelist: Path) -> int: + """Write the flagged sentences the whitelist holds back to the given whitelist file, sorted and one per line. A sentence several files hold is written once, so regenerating the file rewrites the lines that changed - rather than reshuffling all of them. + rather than reshuffling all of them. It leaves out a sentence the whitelist does not hold yet, so that sentence + has to be rewritten to pass the check. """ - sentences = {_normalized(sentence) for _prose, sentence, _faults in flagged} - sys.stdout.writelines(f"{sentence}\n" for sentence in sorted(sentences)) + sentences = {_normalized(sentence) for _prose, sentence, _faults in flagged} & _whitelisted_sentences(whitelist) + whitelist.write_text("".join(f"{sentence}\n" for sentence in sorted(sentences))) return 0 -def _report(flagged: _Flagged, *, checking_whitelist: bool) -> int: +def _check_whitelist(flagged: _Flagged, whitelist: Path) -> int: + """Report each flagged sentence the whitelist does not hold, then each entry the prose does not hold anymore.""" + sentences = list(flagged) + stale = _whitelisted_sentences(whitelist) - {_normalized(sentence) for _prose, sentence, _faults in sentences} + return max(_report(sentences, whitelist), _report_stale(stale, whitelist)) + + +def _report(flagged: _Flagged, whitelist: Path | None) -> int: """Report each flagged sentence the whitelist does not hold, and return 1 where any was reported.""" - whitelisted = _whitelisted_sentences() + whitelisted = _whitelisted_sentences(whitelist) exit_code = 0 - reported = set() for prose, sentence, faults in flagged: - reported.add(_normalized(sentence)) if _normalized(sentence) not in whitelisted: sys.stdout.write(f"{prose.location}: {faults}:\n{textwrap.fill(sentence, width=100)}\n\n") exit_code = 1 - return max(exit_code, _report_stale(whitelisted - reported)) if checking_whitelist else exit_code + return exit_code -def _report_stale(stale: set[str]) -> int: +def _report_stale(stale: set[str], whitelist: Path) -> int: """Report the whitelist entries the run did not match, and return 1 where any was reported.""" for sentence in sorted(stale): - message = f"{_WHITELIST} holds a sentence the prose no longer has, run `just update-whitelists`:" + message = f"{whitelist} holds a sentence the prose no longer has, run `just update-whitelists`:" sys.stdout.write(f"{message}\n{sentence}\n\n") return 1 if stale else 0 diff --git a/tools/vulture-whitelist.py b/tools/vulture-whitelist.py index ec32923..44524c6 100644 --- a/tools/vulture-whitelist.py +++ b/tools/vulture-whitelist.py @@ -7,8 +7,8 @@ tag_last_pushed # unused variable (src/update_time/sources/docker_hub.py:59) download_url # unused variable (src/update_time/sources/github.py:90) database_specific # unused variable (src/update_time/sources/osv.py:52) -upload_time_iso_8601 # unused variable (src/update_time/sources/pypi.py:103) -description_content_type # unused variable (src/update_time/sources/pypi.py:112) +upload_time_iso_8601 # unused variable (src/update_time/sources/pypi.py:101) +description_content_type # unused variable (src/update_time/sources/pypi.py:110) AssertEqualActualFirst # unused class (tools/fixit_rules.py:66) VALID # unused variable (tools/fixit_rules.py:78) INVALID # unused variable (tools/fixit_rules.py:87)