fix(test): remove specs that cannot fail, and fix one whose name lied - #30
Merged
Merged
Conversation
Seven tests removed and one rewritten. Each removal was checked by mutating lib/ and confirming the same defect is still caught -- by a spec that exercises behaviour rather than restating the source. Cannot fail at all: session_spec "constructs without going through Net::HTTP.new's proxy handling" -- `must_be_kind_of Net::HTTP` is true by the class declaration, and `proxy?` returns false in every configuration, including @proxy_from_env forced true with http_proxy set, because the address is localhost. Forcing Session through Net::HTTP.new is caught by the two tests above it, which would fail to construct at all. fake_spec "is a real socket" -- asserts a UNIXSocket responds to read_nonblock, write and to_io. That is Ruby's stdlib. The three tests above it already drive a real exchange through the fake. regression "puts everything under Docker::API" -- Client.name restated from its own declaration. regression "resolves registry credentials per call" -- respond_to? :resolve on a module_function declared three lines away. auth_spec exercises resolve nine times. Strictly duplicated: version_spec "defines no methods on ::Docker" -- same mutation, same catch, as the regression spec's "adds nothing to ::Docker". regression "builds a named-pipe transport from a npipe URL" -- transport_spec covers npipe routing, which is where transport routing lives. regression "declares every query parameter the specification defines" -- reflection over the method signature. Dropping platform from the query hash fails the wire assertion directly above it while this one passes; dropping it from the signature fails both. A strict subset. Superseded: regression "loads no HTTP gem at all" -- greps $LOADED_FEATURES for four named gems, which only populates if the gem is installed. zero_dependency_spec holds every require in lib/ to an allowlist of default gems, catching any of them plus the ones nobody thought to name. Rewritten: regression "defaults to the named pipe on Windows" asserted that DEFAULT_WINDOWS_PIPE contains a substring of itself. Removing the Windows branch from default_url entirely was caught by nothing in the suite -- the name promised routing and the body checked spelling. It now stubs windows? and asserts default_url both ways, against literals rather than against the constants, so a typo in a constant fails too. That is three mutations caught where there was one. 353 runs, 1155 assertions. Signed-off-by: Tim Smith <tim@mondoo.com>
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.
Seven tests removed, one rewritten. Every removal was verified by mutation — break the thing the test claims to guard, confirm the suite still catches it.
353 runs / 1155 assertions, down from 360 / 1179.
Method
For each candidate, mutate
lib/and record which spec files fail:url=setter toClientNotFoundoutside the hierarchycontainer_createdropsplatformfrom the queryplatformfrom the signaturenpipeURLs route toTcp::Dockerdefault_urlignores Windows entirelyTwo mutations that nothing caught before are caught now.
Cannot fail at all
session_spec— "constructs without going through Net::HTTP.new's proxy handling"proxy?isfalsein every configuration — including@proxy_from_envforced true withhttp_proxyset — because the address islocalhost. Verified directly. The regression it names is caught by the two tests above it, which could not construct a session at all.fake_spec— "is a real socket, so it satisfies what Net::BufferedIO asks of one" asserts aUNIXSocketresponds toread_nonblock,write,to_io. That is Ruby's stdlib. The three tests above it already drive a real HTTP exchange through the fake.regression— "puts everything under Docker::API" —Client.namerestated from its own declaration.regression— "resolves registry credentials per call" —respond_to? :resolveon amodule_functiondeclared three lines away in the source.auth_specexercisesresolvebehaviourally nine times.Strictly duplicated
version_spec"defines no methods on ::Docker" — same mutation, same catch, as the regression spec's "adds nothing to ::Docker".regression"builds a named-pipe transport from a npipe URL" —transport_speccovers npipe routing, which is where transport routing belongs.regression"declares every query parameter the specification defines" — reflection over the signature. Droppingplatformfrom the query hash fails the wire assertion directly above it while this one passes; dropping it from the signature fails both. A strict subset.Superseded
regression— "loads no HTTP gem at all" greps$LOADED_FEATURESfor four named gems — which only populates if the gem is actually installed:zero_dependency_specholds everyrequireinlib/to an allowlist of default gems. It catches all four plus the ones nobody thought to name.Rewritten, not removed
regression— "defaults to the named pipe on Windows" was:A constant containing a substring of itself. The name promised routing; the body checked spelling. Deleting the Windows branch from
default_urlwas caught by nothing in the suite.It now stubs
windows?and assertsdefault_urlboth ways — against literals rather than the constants, since comparingdefault_urltoDEFAULT_WINDOWS_PIPEonly proves the two agree with each other. Those literals are an external contract: the pipe Docker Desktop actually publishes.What I checked and kept
Three I expected to cut turned out to earn their place, and the mutations say so:
Client.url=is caught by this test and nothing else.errors_spec"keeps every error under the one root" restates the class hierarchy, but it is the test that names the gem's central promise and gives the clearest failure message.Verification
bundle exec rake— 353 runs, 0 failures.steep checkclean. Chefstyle clean. Full mutation table above re-run against the trimmed suite.