Skip to content

Fix KeyError in crossgen2_comparison.py for omitted_from_diff_dir lookups#126175

Open
Copilot wants to merge 3 commits intomainfrom
copilot/fix-keyerror-in-crossgen2-comparison
Open

Fix KeyError in crossgen2_comparison.py for omitted_from_diff_dir lookups#126175
Copilot wants to merge 3 commits intomainfrom
copilot/fix-keyerror-in-crossgen2-comparison

Conversation

Copy link
Contributor

Copilot AI commented Mar 26, 2026

Description

Crossgen2-comparison outerloop tests intermittently fail with KeyError (e.g. KeyError: 'System.DirectoryServices') when an assembly is present in the base output but missing from the diff output.

Root cause: When generating XML results for assemblies in omitted_from_diff_dir (assemblies in base but not in diff), line 922 looked them up in diff_results_by_name — the dictionary that by definition does not contain them:

# Before (bug): looks up in the wrong dictionary
for assembly_name in omitted_from_diff_dir:
    base_result = diff_results_by_name[assembly_name]

# After (fix): looks up in the correct dictionary
for assembly_name in omitted_from_diff_dir:
    base_result = base_results_by_name[assembly_name]

The adjacent omitted_from_base_dir loop correctly uses diff_results_by_name; this was a copy-paste error in the opposite direction.


📍 Connect Copilot coding agent with Jira, Azure Boards or Linear to delegate work to Copilot in one click without leaving your project management tool.

… omitted_from_diff_dir lookups

The omitted_from_diff_dir set contains assemblies present in the base
directory but not in the diff directory. Line 922 was incorrectly looking
up these assemblies in diff_results_by_name (where they don't exist)
instead of base_results_by_name (where they do exist), causing an
intermittent KeyError when an assembly was omitted from the diff output.

Agent-Logs-Url: https://github.com/dotnet/runtime/sessions/416242cd-0d0e-4de1-bc09-46f242809cd5

Co-authored-by: jtschuster <36744439+jtschuster@users.noreply.github.com>
Copilot AI changed the title [WIP] Fix KeyError in crossgen2-comparison causing outerloop failures Fix KeyError in crossgen2_comparison.py for omitted_from_diff_dir lookups Mar 26, 2026
Copilot AI requested a review from jtschuster March 26, 2026 22:48
@jtschuster jtschuster marked this pull request as ready for review March 26, 2026 22:54
@jtschuster jtschuster requested a review from agocke March 26, 2026 22:56
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Fixes an intermittent KeyError in the crossgen2 comparison outerloop script when generating XML results for assemblies that are present in the base output but missing from the diff output.

Changes:

  • Correct dictionary lookup for omitted_from_diff_dir entries by using base_results_by_name instead of diff_results_by_name.

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@github-actions
Copy link
Contributor

🤖 Copilot Code Review — PR #126175

Note

This review was generated by GitHub Copilot.

Holistic Assessment

Motivation: The PR fixes a clear KeyError bug in crossgen2_comparison.py. The omitted_from_diff_dir set contains assembly names that exist in base but not in diff, so looking them up in diff_results_by_name would always raise a KeyError. The bug is real and verifiable from the code alone.

Approach: The fix is minimal and correct — switching the lookup to base_results_by_name and adding sorted() for deterministic output, matching the pattern used by the adjacent loops (lines 899 and 943).

Summary: ✅ LGTM. This is a straightforward, correct bug fix with no risk of collateral damage.


Detailed Findings

✅ Correctness — Dictionary lookup fix is correct

The core fix changes diff_results_by_name[assembly_name]base_results_by_name[assembly_name] on line 922. This is correct because:

  • omitted_from_diff_dir = base_assemblies - diff_assemblies (line 850) — these assemblies exist in base_assemblies only.
  • base_results_by_name is keyed by assembly names from base_results (line 862), so these names are guaranteed to be present.
  • The old code used diff_results_by_name, which by definition does not contain these assembly names, causing a KeyError whenever omitted_from_diff_dir is non-empty.

I verified the analogous omitted_from_base_dir loop at line 899 correctly uses diff_results_by_name[assembly_name] — no similar bug there.

✅ Consistency — sorted() matches adjacent loops

Adding sorted() to the omitted_from_diff_dir iteration matches the existing pattern in the omitted_from_base_dir loop (line 899) and the both_assemblies loop (line 943, 867). This ensures deterministic XML output ordering.

Generated by Code Review for issue #126175 ·

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

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

3 participants