The refactor that made every validation build pass without validating anything

#testing #python #devops #cicd

Short one. A bug I introduced, caught by reading the diff rather than by any
test, and the reason no test could have caught it.

The setup

A deployment script with a validation mode. On the develop branch it runs with
VALIDATE_ONLY=true and is supposed to check everything without writing
anything.

Original order:

workspace = build_workspace()      # authenticate, resolve, parse

if read_bool("VALIDATE_ONLY"):
    print("Validation-only mode. No workspace changes applied.")
    return

publish_all_items(workspace)

What I did

I was restructuring the function and moved things around for readability. The
result:

if read_bool("VALIDATE_ONLY"):
    print("Validation-only mode. No workspace changes applied.")
    return

workspace = build_workspace()      # now unreachable in validation mode
publish_all_items(workspace)

Looks tidier. Nothing writes to a workspace in either version. Every test passed.

Why it is broken

build_workspace() is not setup code. It is the validation.

Constructing that object authenticates the service principal, resolves the
target workspace over the API, and parses every item in the repository. Most of
what can be wrong is discovered right there.

Return before it and the validation stage prints a reassuring message, exits
zero, and has checked nothing. Every build on the validation branch goes green.
Broken items reach the release branch unnoticed.

Why the tests did not catch it

Every test asserted what the code did: it did not publish, it did not remove
orphans, it exited cleanly. All still true.

Nothing asserted that validation actually validates. There was no test for
"the workspace object gets built before the early return", because that reads
like testing an implementation detail right up until the moment it is the entire
safety property.

What I changed

Restored the order, and left a comment explaining why the line is where it is:

# Built before the validate-only check on purpose. Constructing the
# FabricWorkspace authenticates, resolves the target workspace and parses
# every item, so it is the part that actually validates. Returning before
# this would make develop builds green without checking anything.
workspace = build_workspace()

A comment is a weak guard. But the alternative — a test that asserts an
authentication call happens in a mode defined by not doing anything — is awkward
enough that I would rather the next person read one sentence.

The general version

A refactor can hollow out a safety check while the whole suite stays green, when
the tests assert absence of effects rather than presence of the check.

If you have a validate-only or dry-run mode, ask what in it is actually doing
the validating, and whether anything would notice if that stopped happening.

Full context: [https://www.linkedin.com/pulse/building-enterprise-microsoft-fabric-cicd-practical-guide-mintu-ghosh-f3dbf/]

Code: [https://github.com/vedaforge-team/fabric-cicd-reference/tree/v1.0.0]

DEV Community