Skip to content

Conversation

Copy link
Contributor

Copilot AI commented Oct 31, 2025

Description

Migrated unit tests to use [DataRow] attributes following the pattern established in PR #186. This eliminates test data classes and foreach loops in favor of parameterized test methods, making tests cleaner and more maintainable.

NFUnitTestSystemLib (9 files) - Migrated Equals() tests for all primitive numeric types:

  • Replaced TestData helper classes with [DataRow] attributes
  • Split type-mismatch edge cases (null, string, int) into separate test methods
  • Applied to: Byte, SByte, Int16, Int32, Int64, UInt16, UInt32, UInt64, Boolean

NFUnitTestBitConverter - Migrated 4 GetBytes* test methods:

  • GetBytesBool, GetBytesChar, GetBytesInt16, GetBytesInt32
  • Extracted min/max value tests into separate methods where applicable

Motivation and Context

Addressing nanoframework/Home#1159.

Following the pattern established in PR #186 to keep the code clean and allow for easier addition of new test cases in the future.

How Has This Been Tested?

Screenshots

N/A - No UI changes

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.
Original prompt

This section details on the original issue you should resolve

<issue_title>Migrate Unit Tests to DataRow</issue_title>
<issue_description>### Details about Problem

nanoFramework area: ( CoreLibrary)

Description

To keep the code clean we should migrate nanoframework/CoreLibrary Unit Tests to use DataRow. In a future it will allow us to add more test cases to each method.
Please check this #186 PR for reference.

</issue_description>

<agent_instructions>Make sure to use PR template on the repo and fill in relevant sections. Only tick check lists, don't delete them.</agent_instructions>

Comments on the Issue (you are @copilot in this section)


💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

@coderabbitai
Copy link

coderabbitai bot commented Oct 31, 2025

Important

Review skipped

Bot user detected.

To trigger a single review, invoke the @coderabbitai review command.

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


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

Copilot AI and others added 6 commits October 31, 2025 10:18
Co-authored-by: josesimoes <1881520+josesimoes@users.noreply.github.com>
Co-authored-by: josesimoes <1881520+josesimoes@users.noreply.github.com>
Co-authored-by: josesimoes <1881520+josesimoes@users.noreply.github.com>
Co-authored-by: josesimoes <1881520+josesimoes@users.noreply.github.com>
Co-authored-by: josesimoes <1881520+josesimoes@users.noreply.github.com>
@josesimoes
Copy link
Member

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

Copilot AI changed the title [WIP] Migrate unit tests to use DataRow Migrate unit tests to DataRow pattern Oct 31, 2025
Copilot AI requested a review from josesimoes October 31, 2025 10:30
@josesimoes
Copy link
Member

@copilot Make sure to use PR template on the repo and fill in relevant sections. Only tick check lists, don't delete them.

Copy link
Contributor Author

Copilot AI commented Oct 31, 2025

@copilot Make sure to use PR template on the repo and fill in relevant sections. Only tick check lists, don't delete them.

Updated the PR description to follow the template format with all required sections and checklists.

Copilot finished work on behalf of josesimoes October 31, 2025 10:46
@josesimoes
Copy link
Member

/azp run

@josesimoes josesimoes marked this pull request as ready for review October 31, 2025 10:59
@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

Copy link
Member

@josesimoes josesimoes left a comment

Choose a reason for hiding this comment

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

LGTM

@nfbot
Copy link
Member

nfbot commented Oct 31, 2025

Hi @Copilot,

😯 I'm afraid you'll have to use the PR template like the rest of us...
Make sure you've used the template and have include all the required information and fill in the appropriate details. After doing that feel free to reopen the PR. If you have questions we are here to help..

@josesimoes
Copy link
Member

@copilot some of the new tests are not correctly implemented. Follow my fixes in commit d41914b and reproduce on the other ones you've added.

…m UnitTestByte

Co-authored-by: josesimoes <1881520+josesimoes@users.noreply.github.com>
Copy link
Contributor Author

Copilot AI commented Oct 31, 2025

@copilot some of the new tests are not correctly implemented. Follow my fixes in commit d41914b and reproduce on the other ones you've added.

Applied the same assertion pattern from commit d41914b to all numeric type tests (SByte, Int16, Int32, Int64, UInt16, UInt32, UInt64) in commit 880b0ce. Changes include:

  • Using Assert.AreEqual/Assert.AreNotEqual instead of Assert.AreEqual(expected, ...)
  • Using Assert.IsTrue/Assert.IsFalse for hash code comparisons
  • Removing error messages from assertions
  • Adding blank lines for formatting

Copilot AI requested a review from josesimoes October 31, 2025 12:24
Copilot finished work on behalf of josesimoes October 31, 2025 12:24
@josesimoes
Copy link
Member

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

Copy link
Member

@josesimoes josesimoes left a comment

Choose a reason for hiding this comment

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

LGTM

@sonarqubecloud
Copy link

@josesimoes josesimoes merged commit acfc7d3 into main Oct 31, 2025
7 checks passed
@josesimoes josesimoes deleted the copilot/migrate-unit-tests-to-datarow branch October 31, 2025 13:17
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.

Migrate Unit Tests to DataRow

3 participants