Repository navigation
Write down what the green tests taught this cycle - #95
Merged
Merged
Conversation
Three rules, each of which cost a defect that a passing test did not see: - a word being present is not a criterion. pgrep matched the backend detection, named-checkconf matched an echo string; both mutants stayed green until the checks asserted the property instead of the mention - GNU extensions break silently on BSD. sed -i, \? and i\ with a tab produce different output on macOS without erroring, which is how 'talso-notify' and a surviving quoted "none" got written - every enumeration needs a lower bound. Looking for sendEmail found 15 handlers and skipped 3 that send through sendEmailNoArchive Also says why fixtures/cpanel-named.conf is kept verbatim: three views and a logging block are the shape that breaks, and a tidied copy stops reproducing it.
Collaborator
Author
|
Ready to merge. Three rules, each anchored to a case from this cycle where a green test missed broken code, and the fixture note says why cpanel-named.conf must stay as taken from the server; no upstream names, no attribution, CI green. |
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.
Three rules for
hosting-panels/common/tests/, each of which cost a real defect that a passing test did not see. Suggested in review of #93; the next hook will be written by someone who was not here for these.A word being present is not a criterion
test_pdns_restart.shchecked thatpgrep -x pdns_serverappeared in the installer. It does — in the backend detection, ten lines above the restart. Removing the liveness check left the test green.test_named_conf_edit.shchecked thatnamed-checkconfappeared. It does — inside the message"Check: named-checkconf". Replacing the real guard withif trueleft the test green.Both now assert the property:
if named-checkconfgating the branch, a rollback restoring the backup, adid not come backmessage on the path where the service stayed down.GNU extensions break silently on BSD
sed -iwithout an argument,\?,i\with a leading tab — GNU-only. On macOS they do not fail, they produce different output: the oldnamed.confedit wrotetalso-notifyand left a quoted"none"in place. A test that runs the code catches this; a test that greps it cannot.Every enumeration needs a lower bound
TestEveryHandlerRendersHTMLlooked forsendEmail, found 15 handlers, and the bound of 18 caught that three more send throughsendEmailNoArchiveand were being skipped entirely.checked >= 3in two shell tests exists for the same reason.Fixtures
Also records why
fixtures/cpanel-named.confis kept exactly as it came off the server: three views and a logging block are the shape that broke, and a tidied copy would stop reproducing the failure.Every claim in the file was checked against the code before committing — the
if named-checkconfguard, bothchecked >= 3bounds,minHandlers = 18, the threeviewblocks in the fixture, and theawkinsert.