Skip to content

fix(task): drop the space after the comma in two mutation-exclusion directives - #88

Closed
easonLiangWorldedtech wants to merge 1 commit into
mainfrom
u5a-u4-directive-comma-space
Closed

easonLiangWorldedtech wants to merge 1 commit into
mainfrom
u5a-u4-directive-comma-space

Conversation

@easonLiangWorldedtech

Copy link
Copy Markdown
Owner

Two mutation-exclusion comments in src/core/task/Task.ts are rejected by the mutation gate in scripts/stryker-diff.mjs, and they are rejected for a space.

The gate validates every Stryker disable directive in a changed file against this pattern, taken from the gate blob 7dfe0006d2d5c7e4d2282f65f68b8ed5f16866bc at scripts/stryker-diff.mjs line 87, and the pattern allows no whitespace after a comma inside the mutator list:

const DISABLE_DIRECTIVE =
	/^\s*\/\/\s*Stryker disable next-line ([A-Za-z][A-Za-z0-9]*(?:,[A-Za-z][A-Za-z0-9]*)*)\s*:\s*(\S.*)$/

The two comments read ConditionalExpression, EqualityOperator and ConditionalExpression, LogicalOperator, EqualityOperator. Because of the space after the comma the whole line fails to match, so validateDisableDirectives reports the comment as a broad or unreasoned exclusion even though it names its mutators and gives a reason. Measured with the gate's own pattern against the file at main tip 53cb5b5, blob 77a1333b39e0d265e05f9391e385ed3ba8cfe416, 7 directive lines, 5 accepted, 2 rejected, at lines 4454 and 4890. Measured the same way at this head, blob b5debfe608d65be6e1257d811aad7afd1e2b309f, 7 directive lines, 7 accepted, 0 rejected, and the gate's own validateDisableDirectives invoked on the whole-file line set returns cleanly instead of throwing.

This matters beyond this file because the advisory a run prints for a pull that touches Task.ts is caused by these two comments rather than by the pull, and because the validator throws at the first bad directive and the caller catches once per file, a printed advisory count is a lower bound and not a count. The run against a pull that touches this file printed 1 advisory naming line 4454 while a second directive at line 4890 fails for the same reason and was invisible.

The change is 3 characters across 2 comment lines, the 2 lines the diff shows, no code line, no lint suppression file, no changeset, no changelog. The instrument at this head against its parent prints merge base 53cb5b5, 0 package rows, 0 files and 0 counted executable lines with 0 advisories, which is the expected print for a comment-only change and is reported as an empty manifest rather than as a pass, since a manifest naming 0 files is a refusal to price and not a measurement.

CI is the only proof of the gate, and a local preflight here is not evidence in either direction on this machine.
Where this pull sits, stated rather than left to be inferred. The automation credential that pushed the branch is allowed to push to the fork and to comment on the org repo, and it is not allowed to write a pull request to the org repo, which the attempt showed as a 401 with the message Requires authentication. So this pull is opened against the fork's main, and the fork's main is the same commit as the org repo's main, 53cb5b5, which is the base this change was cut from and the base it is meant to land on. Converting it into a pull against the org repo needs a credential that may write there.

…irectives

The gate in scripts/stryker-diff.mjs validates every Stryker disable directive in a
changed file and rejects one whose mutator list does not match its pattern,
which permits no whitespace after the comma inside the list. Two directives in
this file read ConditionalExpression, EqualityOperator and ConditionalExpression,
LogicalOperator, EqualityOperator, so the whole comment fails to match and is
reported as a broad or unreasoned exclusion even though each names its mutators
and gives a reason.

Removing the space after each comma in those two comments is the whole change,
three characters across two comment lines, with no code line, no suppression
file, no changeset and no changelog touched.
@easonLiangWorldedtech

Copy link
Copy Markdown
Owner Author

Correction to the paragraph in the body of this pull.

The body says the automation credential is allowed to push to the fork and to comment on the org repo and is not allowed to write a pull request to the org repo, citing a 401 with the message Requires authentication. That attribution is wrong and it is ours to correct. The 401 came from a malformed request shape on our side, the authorization fields passed where the HTTP client expects a headers entry, so the request went anonymous, and an anonymous request to a write endpoint is refused on authentication grounds before any permission is consulted. The same credential then opened Zoo-Code-Org#2428 against Zoo-Code-Org/Zoo-Code with the same title, head branch, base and body.

This pull is closed as redundant for that reason and not because anything about the change changed. It carries the identical head 4f0ca7c against the identical base 53cb5b5 for the identical 1 file with 2 insertions and 2 deletions as pull 2428, and its own check-runs read returned total_count 0, so no CI arm ever ran against this location, while the org repo reports 17 check runs for the same commit. The location of a pull decides whether CI sees it, and the upstream location is the one CI reads.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant