Fixes a runbook test's sabotage note - #123
Merged
Merged
Conversation
The note above "starting both vaults inside the callback, from configuration, makes it a clean pass" named a mutation of Pass.migratable/1, which every dry-run test would catch and which says nothing about the vaults. It now names the mutation that breaks what the test claims: the fixture vault built its provider over an empty workspace map instead of the configured one, so the vault started inside the callback held no key for the row's scope and the pass answered :error. Run against this test, the test failed on assert status == :ok. Test comment only; no library change and no fragment. Refs: ece-5g8
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
The runbook test "starting both vaults inside the callback, from configuration, makes it a clean pass" (
test/encryptor/ecto/runbook_test.exs, describe "a release task that starts only the repository") carried a sabotage note that mutatedPass.migratable/1. That mutation breaks every dry-run count, so it says nothing about the property this test claims: that vaults started inside the release task's callback, reading their keys from configuration, give a clean pass.The note now names a mutation of that property: the fixture
Vault.init/1(test/support/test_runbook.ex,Encryptor.Ecto.TestRunbook.Vault) builds its provider over an empty workspace map instead of the configured one. The vault still starts inside the callback, holds no key for the row's scope, and the pass answers:error.Test comment only: no library change, no guide change, no changelog fragment (
changelog.d/README.mdexcludes test harness changes). The bead's other half, the release-task "If it differs" paragraph of the migrate-from-cloak guide, was fixed in an earlier PR and is not touched here.Sabotage run
Encryptor.Ecto.TestRunbook.Vault.init/1,provider(Keyword.fetch!(keys, :workspaces), ...)replaced withprovider(%{}, ...);git diffof the fixture was non-empty.assert status == :ok(left:error), a kill.cmpandgit diff --exit-code, then recompiled; the runbook file passed again.Review (in-turn)
I re-read the diff against the bead's acceptance and notes. Every claim in the new comment was checked against the code:
Vault.init/1builds its provider from the:workspacesentry of the application environment it reads at start; the mutation leaves the vault startable, so the test reaches its assertions rather than failing on the{:ok, _vault}match; the assertion that fails isstatus == :ok. The comment keeps the file's sabotage-note shape (mutation, then consequence, after a hyphen). The sibling test file for the parallel-column exit is untouched.Gate
Full
mix qualitygreen on the rebased head with the database arm running (no skip message).Refs: ece-5g8