1030637: Improved InputDateType Test Coverage in Blazor #67019
Open
irfanajaffer wants to merge 6 commits into
Open
1030637: Improved InputDateType Test Coverage in Blazor #67019irfanajaffer wants to merge 6 commits into
irfanajaffer wants to merge 6 commits into
Conversation
irfanajaffer
changed the title
irfanajaffer
changed the title
irfanajaffer
changed the title
This was referenced Jun 4, 2026
lewing
added a commit
to lewing/aspnetcore
that referenced
this pull request
Jun 5, 2026
Six unique findings from the Copilot reviewer (each posted twice across
two review passes):
1-3. Missing .lock.yml files for code-review, evaluate-pr-tests, learn-from-pr
→ Compiled via 'gh aw compile'. Lock files + .github/aw/actions-lock.json
delta now included.
4-5. evaluate-pr-tests.md gate exits 1 for legitimate skip cases (PR not OPEN;
no test files in diff), causing the workflow run to show as failed.
→ Changed to 'exit 0' with ::notice:: emission. The downstream agent
still activates and calls 'noop' when there's no work — one wasted
activation per skip is acceptable cost to avoid false CI failures.
Genuine infra failures (gh API errors) still exit 1.
6. validate-pat-pool.yml summary text said empty pool would 'fall back to
COPILOT_GITHUB_TOKEN' but the workflow exits 1 on empty pool.
→ Reworded the empty-pool summary to acknowledge both behaviors:
runtime falls back (correctly), but the validator surfaces empty as a
failure to prompt operators to seed the pool. Kept exit 1 — alerting
on empty pool is the desired behavior.
Additional fixes uncovered by compilation:
- learn-from-pr.md: dropped 'imports: shared/pat_pool.md' and the engine
PAT-pool case() expression. The shared pat_pool job needs a pre_activation
job that workflow_dispatch-only triggers don't auto-generate. learn-from-pr
is manually invoked, so default COPILOT_GITHUB_TOKEN is fine (same pattern
as cswin32-update.md).
- evaluate-pr-tests.md: changed workflow_dispatch input pr_number from
'required: true' to 'required: false' — slash_command triggers can't
enforce required manual inputs (gh aw compile rejects this combination).
- evaluate-pr-tests.md: removed 'bots: copilot-swe-agent[bot]' — gh aw warns
that bots + slash_command can cause a bot's comment starting with the
command to occupy the concurrency slot, blocking manual invocations.
SKILL.md improvements (informed by simulating the workflow against real
recently-opened PRs):
- evaluate-pr-tests/SKILL.md: added dimension D0 (Substantive value), weighted
highest. The rubric was rewarding tests that score well on style/convention
even when they duplicated pre-existing coverage or asserted non-existent
invariants. D0 leads the evaluation with 'what does this test exercise that
wasn't covered before, and does the assertion match the source?'
- Added an 'Assertion fabrication' check explicitly: agents must trace
assertions back to the production source line(s) and verify the relationship
before scoring D4 ✅. Documents four common failure modes (cargo-cult
invariant, convenient enumeration, tautology, self-referential setup) at
severity 🚩 above ⚠️ .
- Added 'Coverage-burst pattern recognition' for multi-author 'improve test
coverage' waves common with new-contributor onboarding bursts: extra
scrutiny to D0, look for 'ISSUE #N FIX' comments, don't let headline test
counts overstate real coverage gain, calibrate tone direct rather than
encouraging.
- Added dimension D9 (Consolidation opportunities) — recognize when 5+
near-identical [Fact] tests differing only in input value should be one
[Theory] with [InlineData] rows. Surfaced as the headline finding on a real
simulated PR (PR dotnet#67019, InputDate) where 14 tests collapse cleanly to 2.
- Step 1 now explicitly requires fetching the pre-existing version of each
changed test file (via merge-base) for duplicate detection, and fetching
the production source for assertion-fabrication checks.
lewing
added a commit
to lewing/aspnetcore
that referenced
this pull request
Jun 5, 2026
Six unique findings from the Copilot reviewer (each posted twice across
two review passes):
1-3. Missing .lock.yml files for code-review, evaluate-pr-tests, learn-from-pr
→ Compiled via 'gh aw compile'. Lock files + .github/aw/actions-lock.json
delta now included.
4-5. evaluate-pr-tests.md gate exits 1 for legitimate skip cases (PR not OPEN;
no test files in diff), causing the workflow run to show as failed.
→ Changed to 'exit 0' with ::notice:: emission. The downstream agent
still activates and calls 'noop' when there's no work — one wasted
activation per skip is acceptable cost to avoid false CI failures.
Genuine infra failures (gh API errors) still exit 1.
6. validate-pat-pool.yml summary text said empty pool would 'fall back to
COPILOT_GITHUB_TOKEN' but the workflow exits 1 on empty pool.
→ Reworded the empty-pool summary to acknowledge both behaviors:
runtime falls back (correctly), but the validator surfaces empty as a
failure to prompt operators to seed the pool. Kept exit 1 — alerting
on empty pool is the desired behavior.
Additional fixes uncovered by compilation:
- learn-from-pr.md: dropped 'imports: shared/pat_pool.md' and the engine
PAT-pool case() expression. The shared pat_pool job needs a pre_activation
job that workflow_dispatch-only triggers don't auto-generate. learn-from-pr
is manually invoked, so default COPILOT_GITHUB_TOKEN is fine (same pattern
as cswin32-update.md).
- evaluate-pr-tests.md: changed workflow_dispatch input pr_number from
'required: true' to 'required: false' — slash_command triggers can't
enforce required manual inputs (gh aw compile rejects this combination).
- evaluate-pr-tests.md: removed 'bots: copilot-swe-agent[bot]' — gh aw warns
that bots + slash_command can cause a bot's comment starting with the
command to occupy the concurrency slot, blocking manual invocations.
SKILL.md improvements (informed by simulating the workflow against real
recently-opened PRs):
- evaluate-pr-tests/SKILL.md: added dimension D0 (Substantive value), weighted
highest. The rubric was rewarding tests that score well on style/convention
even when they duplicated pre-existing coverage or asserted non-existent
invariants. D0 leads the evaluation with 'what does this test exercise that
wasn't covered before, and does the assertion match the source?'
- Added an 'Assertion fabrication' check explicitly: agents must trace
assertions back to the production source line(s) and verify the relationship
before scoring D4 ✅. Documents four common failure modes (cargo-cult
invariant, convenient enumeration, tautology, self-referential setup) at
severity 🚩 above ⚠️ .
- Added 'Coverage-burst pattern recognition' for multi-author 'improve test
coverage' waves common with new-contributor onboarding bursts: extra
scrutiny to D0, look for 'ISSUE #N FIX' comments, don't let headline test
counts overstate real coverage gain, calibrate tone direct rather than
encouraging.
- Added dimension D9 (Consolidation opportunities) — recognize when 5+
near-identical [Fact] tests differing only in input value should be one
[Theory] with [InlineData] rows. Surfaced as the headline finding on a real
simulated PR (PR dotnet#67019, InputDate) where 14 tests collapse cleanly to 2.
- Step 1 now explicitly requires fetching the pre-existing version of each
changed test file (via merge-base) for duplicate detection, and fetching
the production source for assertion-fabrication checks.
…o 1030637_InputDateTestRevamp
…najaffer/aspnetcore into 1030637_InputDateTestRevamp
3 tasks
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
1 participant
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.
Description
This PR significantly enhances the test coverage of the Blazor
InputDatecomponent by adding 24 new test cases, increasing the total test count to 26 with 100% pass rate.Key Improvements:
Expanded coverage across all critical areas:
Type support — DateTime, DateTime?, DateTimeOffset, DateTimeOffset?, DateOnly, DateOnly?, TimeOnly, TimeOnly?
InputDateType validation — DateTimeLocal, Month, Time types with appropriate error messages
Display attribute integration — Validation messages use custom display names
ID attribute rendering — Both generated and explicit ID override scenarios
Element assignment — Verifies input element is correctly assigned for all type variants
Nullable type handling — Comprehensive coverage for nullable variants
Type attribute rendering — Verifies
type="time"is rendered for TimeOnlyCustom parsing error messages — Supports custom error message templates
ValueChanged callback — Validates callback is invoked on value changes
CSS class updates — Valid, modified, invalid states based on validation
Name attribute rendering — Verifies name attribute is present
AdditionalAttributes propagation — Custom HTML attributes pass through
EditContext integration — Works without EditContext (bind-only scenario)
Event handler binding — onchange handler has valid EventHandler ID
Added strict assertions to ensure regressions are caught
Verified validation messages match expected error format for each InputDateType
Ensured EditContext notifications and validation states are correctly triggered
Covered edge cases for nullable vs non-nullable types
Improved robustness by verifying event binding via handler ID checks
Impact:
PR Checklist