Skip to content

Improve test class discovery#327

Merged
josesimoes merged 2 commits into
developfrom
improve-test-discovery
May 7, 2026
Merged

Improve test class discovery#327
josesimoes merged 2 commits into
developfrom
improve-test-discovery

Conversation

@josesimoes
Copy link
Copy Markdown
Member

Description

  • Test-class filtering logic moved to shared code so class scanning now excludes Attribute-derived types before calling type-level custom-attribute reflection.

Motivation and Context

  • Running mscorlib unit tests was consistently causing crashes in the test framework runner. Likely cause from crash dump analysis was the minimal support for attributes parsing in nanoFramework CLR. With this change the test class discovery through attributes is much more targeted without forcing the CLR to parse unsupported named attributes.

How Has This Been Tested?

Screenshots

Types of changes

  • Improvement (non-breaking change that improves a feature, code or algorithm)
  • Bug fix (non-breaking change which fixes an issue with code or algorithm)
  • New feature (non-breaking change which adds functionality to code)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Config and build (change in the configuration and build system, has no impact on code or features)
  • Dependencies (update dependencies and changes associated, has no impact on code or features)
  • Unit Tests (add new Unit Test(s) or improved existing one(s), has no impact on code or features)
  • Documentation (changes or updates in the documentation, has no impact on code or features)

Checklist:

  • My code follows the code style of this project (only if there are changes in source code).
  • My changes require an update to the documentation (there are changes that require the docs website to be updated).
  • I have updated the documentation accordingly (the changes require an update on the docs in this repo).
  • I have read the CONTRIBUTING document.
  • I have tested everything locally and all new and existing tests passed (only if there are changes in source code).
  • I have added new tests to cover my changes.

- Test-class filtering logic moverd to shared code so class scanning now excludes Attribute-derived types before calling type-level custom-attribute reflection.
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 7, 2026

Review Change Stack

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Repository: nanoframework/coderabbit/.coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 5a8aa8a8-eda1-4056-afa6-e927a40d9c9e

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

A new helper method IsTestClassCandidate in Helper.cs filters types by class requirement and excludes attribute types. Test discovery and launcher components now use this helper instead of direct class-type checks for candidate type selection.

Changes

Test Type Candidate Classification

Layer / File(s) Summary
Test Class Candidate Logic
source/TestFrameworkShared/Helper.cs
New public method IsTestClassCandidate(Type type) returns true for class types that are not attribute types. Private helper IsAttributeType(Type type) walks the inheritance chain and compares FullName against System.Attribute.
Type Filtering Adoption
source/TestAdapter/Discover.cs, source/UnitTestLauncher/Program.cs
Test discovery and launcher now invoke Helper.IsTestClassCandidate(type) instead of direct type.IsClass checks when filtering types for test processing.

🎯 2 (Simple) | ⏱️ ~8 minutes

🚥 Pre-merge checks | ✅ 4
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The pull request title 'Improve test class discovery' is concise (28 characters, well under 50), descriptive, and accurately summarizes the main change—enhancing test class discovery by filtering for non-Attribute types.
Description check ✅ Passed The pull request description is directly related to the changeset, explaining the filtering logic moved to shared code, the motivation (avoiding CLR crashes with attribute parsing), and how the change improves test class discovery.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@josesimoes
Copy link
Copy Markdown
Member Author

@coderabbitai review

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 7, 2026

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@source/TestFrameworkShared/Helper.cs`:
- Around line 25-38: IsAttributeType currently walks current.BaseType and reads
current.FullName without handling TypeLoadException; in environments where
resolving base types can fail (desktop-reflection), this can yield false
negatives—wrap the BaseType traversal in a try/catch that catches
TypeLoadException (and any specific reflection exceptions you see) when
accessing current.BaseType or current.FullName and on catch conservatively treat
the inspected type as an attribute type (i.e., return true) so failing to
resolve a base type won’t incorrectly classify an attribute-derived type as a
test class; update the IsAttributeType method accordingly.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository: nanoframework/coderabbit/.coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 354eb471-92fc-43dd-bf4b-0a13e450d561

📥 Commits

Reviewing files that changed from the base of the PR and between 6aa00b2 and db83e09.

📒 Files selected for processing (3)
  • source/TestAdapter/Discover.cs
  • source/TestFrameworkShared/Helper.cs
  • source/UnitTestLauncher/Program.cs

Comment thread source/TestFrameworkShared/Helper.cs
@josesimoes josesimoes merged commit 9d8b809 into develop May 7, 2026
7 checks passed
@josesimoes josesimoes deleted the improve-test-discovery branch May 7, 2026 13:22
@josesimoes
Copy link
Copy Markdown
Member Author

Same count of tests are now showing and running in both local and pipeline.
image
image

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants