Draft
Conversation
- Publish to nuget now happens on build from develop branch too.
- Temporary disable trigger for tags. ***NO_CI***
- Fix parameter name. - Add branch to publish nugets from. ***NO_CI***
📝 WalkthroughSummary by CodeRabbit
🚥 Pre-merge checks | ✅ 2✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches⚔️ Resolve merge conflicts
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. Comment |
13 tasks
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
azure-pipelines.yml (2)
59-60: Remove duplicate variable declaration & trailing spaces.You’ve declared
DOTNET_NOLOGOboth globally (lines 36–37) and again under theBuild_mscorlibjob. This duplication is unnecessary and may lead to confusion. Also, YAMLlint flagged trailing spaces on line 60.🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 60-60: trailing spaces
(trailing-spaces)
132-136: Consider adding thegenericsbranch to release conditions.You’ve broadened the GitHub release condition to run on
mainanddevelop. To stay consistent with the newpublicReleaseRefSpec, you might also includegenericshere if you intend to create releases from that branch.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (11)
.runsettingsis excluded by none and included by noneTests/NFUnitTestGC/TestGC.csis excluded by none and included by noneTests/NFUnitTestSystemLib/UnitTestGCTest.csis excluded by none and included by noneTests/NFUnitTestSystemLib/UnitTestInitLocalTests.csis excluded by none and included by noneTests/NFUnitTestSystemLib/UnitTestReflectionTypeTest.csis excluded by none and included by noneTests/UnitTestLauncher/UnitTestLauncher.nfprojis excluded by none and included by nonenanoFramework.CoreLibrary.nuspecis excluded by none and included by nonenanoFramework.CoreLibrary/System/AssemblyInfo.csis excluded by none and included by nonenanoFramework.CoreLibrary/System/GC.csis excluded by none and included by nonenanoFramework.CoreLibrary/System/Guid.csis excluded by none and included by nonenanoFramework.TestFrameworkis excluded by none and included by none
📒 Files selected for processing (2)
azure-pipelines.yml(3 hunks)version.json(2 hunks)
🧰 Additional context used
🪛 YAMLlint (1.35.1)
azure-pipelines.yml
[error] 60-60: trailing spaces
(trailing-spaces)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: nanoframework.CoreLibrary (Build_mscorlib)
- GitHub Check: nanoframework.CoreLibrary (Build_mscorlib)
🔇 Additional comments (4)
version.json (2)
3-3: Approve semantic version update.Updating the base version to
"2.0.0-preview.{height}"correctly switches to semantic versioning with a preview suffix. This aligns with the pipeline change enabling preview builds.
14-14: Approve new public release branch pattern.Adding
^refs/heads/generics$topublicReleaseRefSpecensures the newly merged “generics” branch is considered for public releases.azure-pipelines.yml (2)
79-79: Verify template supportsusePreviewBuild.You’ve enabled
usePreviewBuild: truein theclass-lib-build-only.ymlinvocation. Please confirm that the template accepts and correctly propagates this parameter to invoke preview-mode builds as expected.
122-125: Ensure publish step aligns with new branches.The
class-lib-publish.ymltemplate is now called withbaseBranchName: 'develop'. Given thatversion.jsonincludes agenericsbranch for public releases, please verify whetherbaseBranchNameshould dynamically reflectgenerics(or accept a list of branches) to fully support that branch.
13 tasks
|
…nto develop ***NO_CI***
This was referenced Apr 30, 2025
- Add target for .NET Framework v4.7.2
This was referenced May 8, 2025
This was referenced Jun 5, 2025
This was referenced Jun 17, 2025
Merged
This was referenced Jun 24, 2025
***NO_CI***
- Remove unexistent var from conditions. ***NO_CI***
- Improve download and usage of specific nanoCLR from nf-interpreter build. ***NO_CI***
13 tasks
- Build now uses NuGet cache again. ***NO_CI***
***NO_CI***
This was referenced Dec 11, 2025
- Adjust artifact and path for nanoCLR DLL. ***NO_CI***
- Update project for test framework. ***NO_CI***
- Improve github release task to generate changlog from PRs. ***NO_CI***
|
13 tasks
#267) Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: josesimoes <1881520+josesimoes@users.noreply.github.com> ***NO_CI***
There was a problem hiding this comment.
Actionable comments posted: 21
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
nanoFramework.CoreLibrary/System/Guid.cs (1)
354-385:⚠️ Potential issue | 🟠 MajorValidate the input before touching
input[0].This path throws on
nullandstring.Emptyinstead of returningfalse, and the later length/hyphen checks still only accept the dashed form even though the remarks advertiseDandN.Guid(string)inherits the same behavior through this helper.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@nanoFramework.CoreLibrary/System/Guid.cs` around lines 354 - 385, The TryParse method currently indexes input[0] before validating input and only accepts the dashed form; change it to first guard against null and empty by returning false if input is null or input.Length == 0, then determine format: accept "N" (32 hex digits), "D" (36 with hyphens), and "B" (38 with surrounding braces) by checking lengths first, set startPos to 1 for braces, and only perform hyphen-position checks when handling the "D" or "B" formats; keep creating result as new Guid(new byte[16]) and ensure no input indexing occurs until you've validated the length/format.
♻️ Duplicate comments (1)
azure-pipelines-templates/check-nf-interpreter-to-test.yml (1)
13-18:⚠️ Potential issue | 🟠 MajorFail fast when
GITHUB_TOKENis empty.At Line 15, auth headers are built unconditionally. If
GITHUB_TOKENis missing, the call may proceed unauthenticated and produce flaky behavior.Suggested fix
+ if ([string]::IsNullOrWhiteSpace($env:GITHUB_TOKEN)) + { + throw "GITHUB_TOKEN is required for GitHub API calls." + } + # prepare GitHub API headers using token auth $headers = @{ Authorization = "token $env:GITHUB_TOKEN"Also applies to: 49-50
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@azure-pipelines-templates/check-nf-interpreter-to-test.yml` around lines 13 - 18, The script currently builds the $headers hashtable unconditionally using $env:GITHUB_TOKEN; add a guard that checks that $env:GITHUB_TOKEN is set and non-empty before constructing $headers and abort (Write-Error/throw/exit 1) with a clear message if it is missing, so the pipeline fails fast on missing credentials; apply the same check to the other location that builds $headers/Authorization so both occurrences validate GITHUB_TOKEN before proceeding.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/copilot-instructions.md:
- Around line 28-58: The Markdown fenced code block containing the repository
tree (starting with the line "nanoFramework.CoreLibrary/ # Main BCL
project (with reflection)") is missing a language tag and triggers MD040; fix it
by adding the language identifier `text` immediately after the opening triple
backticks (i.e., replace the opening "```" before the
"nanoFramework.CoreLibrary/" block with "```text") so the directory tree is
properly tagged.
- Around line 9-137: The Markdown has missing blank lines around headings,
tables, and fenced code blocks (MD022/MD058/MD031); fix by ensuring a single
blank line before and after each heading like "### This Is NOT Standard .NET",
around the table under "Two Library Flavours", and before/after fenced code
blocks (the three triple-backtick blocks), and normalize spacing so every
heading, table, and fenced block is separated by exactly one blank line from
surrounding paragraphs or lists throughout the file; scan for repeated
occurrences and apply the same pattern consistently.
In `@azure-pipelines-templates/check-nf-interpreter-to-test.yml`:
- Around line 67-69: The output reference and environment variable are using the
displayName instead of the step name and the wrong env var name; update any
references that point to Flag_availability_of_local_nanoCLR to use the step name
NANOCLR_CHECK (e.g., reference the step outputs via NANOCLR_CHECK instead of the
displayName) and fix the env mapping so the produced env var is
HAS_LOCAL_NANOCLR (or change subsequent uses from
$env:NANOCLR_CHECK_HAS_LOCAL_NANOCLR to $env:HAS_LOCAL_NANOCLR) so the output
and env variable names match the step name and declared output (NANOCLR_CHECK
and HAS_LOCAL_NANOCLR).
In `@azure-pipelines.yml`:
- Around line 74-86: The script reads the wrong env var name; change the
PowerShell script to read $env:HAS_LOCAL_NANOCLR (the variable you inject)
instead of $env:NANOCLR_CHECK_HAS_LOCAL_NANOCLR so the condition that sets
UNIT_TEST_RUNSETTINGS actually uses the mapped value; update the check in that
inline script where HAS_LOCAL_NANOCLR/NANOCLR_CHECK_HAS_LOCAL_NANOCLR are
referenced to use HAS_LOCAL_NANOCLR consistently.
In `@local_clr.runsettings`:
- Line 14: The runsettings file currently hard-codes PathToLocalCLRInstance to
an agent-specific location; change this to a runtime-resolved value so it works
across agents. Replace the static PathToLocalCLRInstance entry in
local_clr.runsettings with a token/placeholder (e.g., %(NANOCLR_PATH) or
${NANOCLR_PATH}) and update the pipeline/job that downloads the artifact to set
that token to the actual download directory (or write the resolved path into the
file as a pipeline step) so PathToLocalCLRInstance is populated at runtime
rather than being fixed to D:\a\_temp\...\nanoFramework.nanoCLR.dll.
In `@nanoFramework.CoreLibrary.Benchmarks/Program.cs`:
- Line 24: Change the implicitly typed declaration of coreLibVersion to an
explicit System.Version type: replace the use of var with System.Version for the
variable named coreLibVersion in Program (the assignment from
typeof(object).Assembly.GetName().Version) so the code complies with the C#
coding standard.
In `@nanoFramework.CoreLibrary.Benchmarks/Properties/AssemblyInfo.cs`:
- Around line 11-16: Assembly metadata still uses the template name
"CSharp.BlankApplication"; update the AssemblyTitle and AssemblyProduct
attributes in AssemblyInfo.cs to the correct project name (replace existing
"CSharp.BlankApplication" values) so they reflect this project (use
"nanoFramework.CoreLibrary.Benchmarks" or the canonical project product/title),
leaving other attributes unchanged unless additional project metadata should be
updated.
- Around line 35-36: Remove the hardcoded assembly attributes by deleting the
lines that declare AssemblyVersion and AssemblyFileVersion in AssemblyInfo.cs
(the [assembly: AssemblyVersion("1.0.0.0")] and [assembly:
AssemblyFileVersion("1.0.0.0")] entries) so they don’t override
Nerdbank.GitVersioning; ensure versioning is produced by your
version.json/Nerdbank.GitVersioning config and that no duplicate
AssemblyVersion/AssemblyFileVersion attributes exist elsewhere in the project.
In `@nanoFramework.CoreLibrary/CoreLibrary.nfproj`:
- Line 81: The MSBuild include for the InAttribute source is mis-typed as
"InAttribute .cs" with an extra space; update the <Compile
Include="System\Runtime\InteropServices\InAttribute .cs" /> entry to remove the
space so it matches the actual file name (InAttribute.cs) so the compiler can
locate the System\Runtime\InteropServices\InAttribute.cs source file.
- Around line 69-81: There is profile drift between CoreLibrary.nfproj and
CoreLibrary.NoReflection.nfproj: confirm whether the exclusion of the listed
compile items (System\Array.Enumerators.cs,
System\ArrayTypeMismatchException.cs, System\Collections\Generic\ICollection.cs,
System\Collections\Generic\ICollectionDebugView.cs,
System\Collections\Generic\IEnumerable.cs,
System\Collections\Generic\IEnumerator.cs, System\MemoryExtensions.cs,
System\Nullable.cs, System\ReadOnlySpan*.cs, System\Span*.cs,
System\Runtime\CompilerServices\Unsafe.cs,
System\Runtime\InteropServices\InAttribute .cs) from
CoreLibrary.NoReflection.nfproj is intentional; if intentional, add a brief
documented comment in the NoReflection project (or repo README) stating which
APIs are omitted and why; if unintentional, add the missing <Compile
Include="..."/> entries to CoreLibrary.NoReflection.nfproj to match
CoreLibrary.nfproj (use the same file names/paths as in CoreLibrary.nfproj) and
run a build to verify; finally, normalize CoreLibrary.nfproj XML indentation to
2 spaces per project coding guidelines (adjust element indentation throughout
the file).
In `@nanoFramework.CoreLibrary/System/Array.cs`:
- Around line 356-365: Rename the internal static field Value inside the nested
EmptyArray<T> class to s_value to follow the s_camelCase convention; update the
Empty<T>() method (and any other references) to return EmptyArray<T>.s_value and
ensure the pragma warning lines remain around the field declaration in
EmptyArray<T> to preserve suppression of CA1825/IDE0300.
In `@nanoFramework.CoreLibrary/System/Guid.cs`:
- Around line 181-190: CompareTo(Guid) currently compares the packed _data int[]
entries directly which alters GUID component order and risks overflow; change
CompareTo(Guid) to unpack the GUID into its per-byte fields (a, b, c, d, e, f,
g, h, i, j, k) in GUID sort order and compare each component sequentially,
returning -1/0/1 (not a subtraction) as soon as a difference is found; update
usages of _data (and initialization logic that sets _data) inside CompareTo to
derive those components before comparing.
In `@nanoFramework.CoreLibrary/System/ReadOnlySpan.cs`:
- Around line 237-252: The operator == currently does element-wise comparison
but per the spec it must test span identity: modify ReadOnlySpan<T>.operator ==
to return true only when Lengths are equal and the spans start at the same
memory location; implement this by comparing the start reference/pointer (e.g.,
use MemoryMarshal.GetReference(left) == MemoryMarshal.GetReference(right)) or by
comparing the span's internal start fields (such as _pinnable/_byteOffset or
_pointer) along with Length, and ensure operator != (if present) continues to be
the negation of this identity comparison.
- Around line 315-319: The ToArray() method currently copies from the raw
backing _array at index 0 which ignores the span's slice semantics and can fail
for null-backed empty spans; change ToArray() in ReadOnlySpan<T> to allocate the
destination T[] of length _length and populate it by calling CopyTo(new
Span<T>(destination)) so the span's start/length and null-backed empty cases are
respected (use the existing CopyTo(Span<T>) implementation rather than
Array.Copy on _array).
In `@nanoFramework.CoreLibrary/System/Span.cs`:
- Around line 22-23: Span<T> currently only stores _array and _length so it
cannot represent slices or pointer-backed spans; update the internal state to
preserve a base offset/pointer and adjust constructors and members to use it:
add a readonly int _start (or _offset) and a readonly IntPtr/_ptr field to hold
native pointer info, update the public constructors (the ones accepting start
and void*) to initialize these new fields, and modify this[int], Clear(),
Slice(), ToArray(), operator==, and the ReadOnlySpan<T> conversion to compute
indices and lengths using _start/_ptr as appropriate so sliced and
pointer-backed spans are correctly represented (refer to Span<T> constructors
and methods named this[int], Clear, Slice, ToArray, op_Equality, and the
ReadOnlySpan<T> conversion).
- Around line 31-47: The struct constructors for Span<T> call the instance
method NativeSpanConstructor(...) before this is definitely assigned; to fix,
assign this = default; at the very start of each constructor (both the T[]?
overload and the other constructor referenced around lines 87-119) so the struct
is definitely initialized before any instance method invocation, then proceed
with the null check, type check, and call to NativeSpanConstructor; update both
constructors (the one taking T[]? and the other constructor mentioned)
accordingly.
In `@nanoFramework.CoreLibrary/System/String.cs`:
- Around line 49-52: In String.Equals(object obj) replace the implicitly typed
local 'var s = obj as string;' with an explicit type declaration 'string s = obj
as string;' so the Equals method uses an explicitly typed local; update the line
in the Equals method (the local named s) accordingly.
- Around line 61-70: In the NANOCLR_REFLECTION branch update the signature of
the extern method Equals so it accepts nullable inputs—change public static
extern bool Equals(string a, string b) to use nullable parameters (string? a,
string? b) to match the public operators ==(string? a, string? b) and !=(string?
a, string? b); locate the method definition in System/String.cs inside the `#if`
NANOCLR_REFLECTION block and adjust the parameter types accordingly so callers
can pass null without warnings.
In `@Tests/NFUnitTestTypes/UnitTestGuid.cs`:
- Around line 15-137: Replace all implicit var declarations in this test file
with explicit types to follow the repo C# style: e.g. in methods
Guid_Empty_IsAllZeros, Guid_Constructor_ByteArray_RoundTrip,
Guid_Equals_And_Operator, Guid_NotEquals_And_Operator, Guid_ToString_And_Parse,
Guid_GetHashCode_Consistent, Guid_CompareTo_Object_And_Self,
Guid_TryParseGuidWithDashes_Valid, Guid_TryParseGuidWithDashes_Invalid,
Guid_Constructor_String_WithBraces, Guid_Constructor_String_Invalid_Throws and
Guid_GetHashCode_Empty change usages like "var empty", "var bytes", "var
original", "var roundTrip", "var g1", "var g2", "var str", "var parsed" to their
explicit types (Guid, byte[], bool, string, etc.); update all out variables
(e.g. Guid.TryParse(..., out var g2)) to use explicit type declarations or
declare the variable on a separate line before the call so no var remains.
- Around line 77-138: Add tests to cover non-happy-path ordering and additional
parsing formats: create new tests that assert Guid_CompareTo_Order where two
different GUIDs produce non-zero CompareTo results in both directions (e.g.,
verify g1.CompareTo(g2) is <0 and g2.CompareTo(g1) is >0) and that
CompareTo((object)null) returns 1 (reference Guid_CompareTo_Object_And_Self).
Add TryParse edge-case tests: Guid_TryParse_NullOrEmpty_Invalid which calls
Guid.TryParse(null, out var) and Guid.TryParse("", out var) and asserts false
and Guid.Empty, and Guid_TryParse_AlternateFormats_Valid to TryParse the same
GUID formatted as "N", "B", and "P" strings and assert success and equality (to
complement Guid_TryParseGuidWithDashes_Valid). Also add a
Guid_ParseOrCtor_ParenthesesAndNoDashes test to construct/parse strings with
parentheses and the "N" format and assert equality, and a
Guid_Parse_Invalid_Empty_Throws or equivalent to confirm Parse/constructor
behavior for empty string if applicable; update or add tests named above
(Guid_CompareTo_Order, Guid_TryParse_NullOrEmpty_Invalid,
Guid_TryParse_AlternateFormats_Valid, Guid_ParseOrCtor_ParenthesesAndNoDashes)
accordingly.
In `@Tests/NFUnitTestTypes/UnitTestsReadOnlySpanByte.cs`:
- Around line 699-986: Tests assume content-based equality but the library
implements identity-based equality in ReadOnlySpan/Span; update the test methods
(e.g., EqualityOperatorTests, EqualityWithPartialSpansTests,
EqualityWithSlicedSpansTests, EqualityWithStackAllocatedSpansTests,
EqualityContentBasedTests, etc.) to assert content equality using a content
comparer rather than the ==/!= operators — for example replace
Assert.IsTrue(span1 == span2) / Assert.IsFalse(span1 != span2) with
Assert.IsTrue(MemoryExtensions.SequenceEqual(span1, span2) or the equivalent
SequenceEqual extension) and replace inequality checks accordingly; keep using
==/!= only when you intend to assert identity/reflexivity (e.g., span == span)
so the tests match ReadOnlySpan.cs / Span.cs semantics.
---
Outside diff comments:
In `@nanoFramework.CoreLibrary/System/Guid.cs`:
- Around line 354-385: The TryParse method currently indexes input[0] before
validating input and only accepts the dashed form; change it to first guard
against null and empty by returning false if input is null or input.Length == 0,
then determine format: accept "N" (32 hex digits), "D" (36 with hyphens), and
"B" (38 with surrounding braces) by checking lengths first, set startPos to 1
for braces, and only perform hyphen-position checks when handling the "D" or "B"
formats; keep creating result as new Guid(new byte[16]) and ensure no input
indexing occurs until you've validated the length/format.
---
Duplicate comments:
In `@azure-pipelines-templates/check-nf-interpreter-to-test.yml`:
- Around line 13-18: The script currently builds the $headers hashtable
unconditionally using $env:GITHUB_TOKEN; add a guard that checks that
$env:GITHUB_TOKEN is set and non-empty before constructing $headers and abort
(Write-Error/throw/exit 1) with a clear message if it is missing, so the
pipeline fails fast on missing credentials; apply the same check to the other
location that builds $headers/Authorization so both occurrences validate
GITHUB_TOKEN before proceeding.
🪄 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: 13a40bf3-c2b0-4108-8df3-63bbd8bb2943
📒 Files selected for processing (33)
.github/copilot-instructions.mdTests/NFUnitTestSystemLib/NFUnitTestSystemLib.nfprojTests/NFUnitTestSystemLib/UnitTestInitLocalTests.csTests/NFUnitTestSystemLib/UnitTestMemoryExtensions.csTests/NFUnitTestTypes/NFUnitTestTypes.nfprojTests/NFUnitTestTypes/UnitTestGuid.csTests/NFUnitTestTypes/UnitTestsReadOnlySpanByte.csTests/NFUnitTestTypes/UnitTestsSpanByte.csTests/TestFramework/TestFramework.nfprojazure-pipelines-templates/check-nf-interpreter-to-test.ymlazure-pipelines.ymllocal_clr.runsettingsnanoFramework.CoreLibrary.Benchmarks.slnnanoFramework.CoreLibrary.Benchmarks/Program.csnanoFramework.CoreLibrary.Benchmarks/Properties/AssemblyInfo.csnanoFramework.CoreLibrary.Benchmarks/ToString/ToStringConcatenation.csnanoFramework.CoreLibrary.Benchmarks/ToString/ToStringFormat.csnanoFramework.CoreLibrary.Benchmarks/ToString/ToStringInterpolation.csnanoFramework.CoreLibrary.Benchmarks/ToString/ToStringPlain.csnanoFramework.CoreLibrary.Benchmarks/nanoFramework.CoreLibrary.Benchmarks.nfprojnanoFramework.CoreLibrary.Benchmarks/packages.confignanoFramework.CoreLibrary/CoreLibrary.nfprojnanoFramework.CoreLibrary/System/Array.csnanoFramework.CoreLibrary/System/AssemblyInfo.csnanoFramework.CoreLibrary/System/Collections/Generic/ICollection.csnanoFramework.CoreLibrary/System/Collections/Generic/ICollectionDebugView.csnanoFramework.CoreLibrary/System/Guid.csnanoFramework.CoreLibrary/System/MemoryExtensions.csnanoFramework.CoreLibrary/System/ReadOnlySpan.csnanoFramework.CoreLibrary/System/Runtime/CompilerServices/Unsafe.csnanoFramework.CoreLibrary/System/Span.csnanoFramework.CoreLibrary/System/String.csnanoFramework.TestFramework
💤 Files with no reviewable changes (1)
- Tests/TestFramework/TestFramework.nfproj
Comment on lines
+9
to
+137
| ### This Is NOT Standard .NET | ||
| - The code targets embedded systems (MCUs) with severe memory and flash constraints. | ||
| - Many methods are implemented in native C++ ("native stubs") and are declared with `[MethodImpl(MethodImplOptions.InternalCall)]` — **do not add a body to these methods**. | ||
| - Some standard .NET APIs are intentionally unsupported (throw `NotSupportedException`) to preserve assembly/image size. Do not remove these stubs; they exist to satisfy interface contracts. | ||
| - The library targets `TargetFrameworkVersion v1.0` (the nanoFramework target, not desktop .NET). | ||
|
|
||
| ### Two Library Flavours | ||
| | Project | Description | NuGet | | ||
| |---|---|---| | ||
| | `nanoFramework.CoreLibrary` | Full BCL **with** `System.Reflection` | `nanoFramework.CoreLibrary` | | ||
| | `nanoFramework.CoreLibrary.NoReflection` | BCL **without** reflection (smaller flash footprint) | `nanoFramework.CoreLibrary.NoReflection` | | ||
|
|
||
| Both produce an assembly named `mscorlib`. The no-reflection variant excludes files under `System/Reflection/` and sets no `NANOCLR_REFLECTION` define. | ||
|
|
||
| ### Project File Format | ||
| Projects use `.nfproj` files (nanoFramework MSBuild project system), not standard `.csproj`. These require Visual Studio with the **nanoFramework VS extension** installed, or MSBuild with `NFProjectSystem.CSharp.targets`. | ||
|
|
||
| ## Repository Structure | ||
|
|
||
| ``` | ||
| nanoFramework.CoreLibrary/ # Main BCL project (with reflection) | ||
| System/ # All System.* source files | ||
| Collections/ # IEnumerable, ArrayList, generic interfaces | ||
| Diagnostics/ # Debug, Debugger, attributes | ||
| Globalization/ # CultureInfo, DateTimeFormatInfo, etc. | ||
| IO/ # IOException | ||
| Reflection/ # Assembly, MethodInfo, FieldInfo, etc. | ||
| Runtime/ # CompilerServices, InteropServices, Remoting | ||
| Threading/ # Thread, Monitor, Timer, WaitHandle, etc. | ||
| CoreLibrary.nfproj | ||
| Directory.Build.props | ||
|
|
||
| nanoFramework.CoreLibrary.NoReflection/ # BCL without reflection | ||
| System/ # Subset of System.* files (no Reflection/) | ||
| CoreLibrary.NoReflection.nfproj | ||
|
|
||
| Tests/ # Unit tests (one folder per test suite) | ||
| NFUnitTestArithmetic/ | ||
| NFUnitTestArray/ | ||
| NFUnitTestBitConverter/ | ||
| NFUnitTestSystemLib/ # Covers most primitive types, strings, DateTime, etc. | ||
| NFUnitTestThread/ | ||
| ... (21 test suites total) | ||
|
|
||
| nanoFramework.TestFramework/ # Git submodule: test framework source | ||
| azure-pipelines.yml # CI/CD pipeline (Azure Pipelines, Windows) | ||
| nanoFramework.CoreLibrary.sln # Main solution (library + tests) | ||
| nanoFramework.CoreLibrary.Benchmarks.sln # Benchmarks solution | ||
| version.json # Nerdbank.GitVersioning configuration | ||
| ``` | ||
|
|
||
| ## Build & Test | ||
|
|
||
| ### Build Requirements | ||
| - **Windows only** — the nanoFramework build toolchain runs on Windows (Azure Pipelines uses `windows-latest`). | ||
| - Visual Studio 2022 with the nanoFramework VS extension, or MSBuild 17+. | ||
| - NuGet packages are restored automatically; `packages.lock.json` enforces locked restore in CI. | ||
|
|
||
| ### Building | ||
| ```powershell | ||
| # Restore NuGet packages first | ||
| nuget restore nanoFramework.CoreLibrary.sln | ||
|
|
||
| # Build the solution (Release) | ||
| msbuild nanoFramework.CoreLibrary.sln /p:Configuration=Release /p:Platform="Any CPU" | ||
|
|
||
| # Build only CoreLibrary (Debug) | ||
| msbuild nanoFramework.CoreLibrary\CoreLibrary.nfproj /p:Configuration=Debug | ||
| ``` | ||
|
|
||
| ### Running Tests | ||
| Tests use the [nanoFramework.TestFramework](https://github.com/nanoframework/nanoFramework.TestFramework) with the **nanoCLR Win32 emulator** (no real hardware needed by default). | ||
|
|
||
| ```powershell | ||
| # From Developer Command Prompt for VS 2022: | ||
| vstest.console.exe .\Tests\NFUnitTestBitConverter\bin\Release\NFUnitTest.dll ^ | ||
| /Settings:.\.runsettings ^ | ||
| /TestAdapterPath:.\nanoFramework.TestFramework\source\TestAdapter\bin\Debug\net4.8 | ||
|
|
||
| # Run all tests using runsettings: | ||
| # .runsettings -> uses preview nanoCLR from NuGet (default) | ||
| # local_clr.runsettings -> uses a locally built nanoCLR | ||
| ``` | ||
|
|
||
| Key runsettings config: | ||
| - `IsRealHardware=False` — runs on emulator | ||
| - `UsePreviewClr=True` — downloads latest preview nanoCLR | ||
| - `MaxCpuCount=1` — tests run sequentially | ||
| - `TestSessionTimeout=1200000` (20 min) | ||
|
|
||
| ### Adding Tests | ||
| - Each test suite is a separate `.nfproj` in `Tests/`. | ||
| - Use `nanoFramework.TestFramework` attributes: `[TestClass]`, `[TestMethod]`, `[Setup]`, `[Cleanup]`, `[DataRow]`. | ||
| - **Do not** reference the NuGet `nanoFramework.TestFramework` — use the project reference from `nanoFramework.TestFramework/` submodule instead (this is required for the CoreLibrary because it replaces mscorlib). | ||
| - **Do not** reference `mscorlib`, `nanoFramework.TestFramework`, or `nanoFramework.UnitTestLauncher` NuGet packages; use project references. | ||
| - Every new API **must** have test coverage for all methods, properties, events, and exceptions. | ||
|
|
||
| ## Coding Conventions | ||
|
|
||
| ### File Header | ||
| Every `.cs` file must begin with: | ||
| ```csharp | ||
| // Licensed to the .NET Foundation under one or more agreements. | ||
| // The .NET Foundation licenses this file to you under the MIT license. | ||
| ``` | ||
|
|
||
| ### Naming | ||
| | Item | Convention | Example | | ||
| |---|---|---| | ||
| | Constants | PascalCase | `MaxValue` | | ||
| | Private/internal fields | `_camelCase` | `_thread` | | ||
| | Private/internal static fields | `s_camelCase` | `s_instance` | | ||
| | Public members | PascalCase | `GetEnumerator` | | ||
|
|
||
| ### Formatting (from `.editorconfig`) | ||
| - Indent: 4 spaces (C#), 2 spaces (XML/project files) | ||
| - Line endings: CRLF | ||
| - Encoding: UTF-8 BOM | ||
| - Always use braces, even for single-line blocks | ||
| - `using` directives outside namespace | ||
| - System directives first | ||
|
|
||
| ### C# Language | ||
| - Language version: C# 13.0 (main project); `default` for NoReflection variant | ||
| - Avoid `var` — use explicit types | ||
| - Prefer expression-bodied members where appropriate | ||
| - `[MethodImpl(MethodImplOptions.InternalCall)]` for native implementations — never add a body | ||
|
|
||
| ### Assembly Size Awareness |
There was a problem hiding this comment.
Fix Markdown structure lint violations (headings/tables/fences spacing).
The file has repeated MD022/MD058/MD031 violations (missing blank lines around headings, tables, and fenced blocks). Please normalize spacing so lint passes consistently.
Suggested pattern to apply across the file
-### Two Library Flavours
-| Project | Description | NuGet |
+### Two Library Flavours
+
+| Project | Description | NuGet |
|---|---|---|
| `nanoFramework.CoreLibrary` | Full BCL **with** `System.Reflection` | `nanoFramework.CoreLibrary` |
| `nanoFramework.CoreLibrary.NoReflection` | BCL **without** reflection (smaller flash footprint) | `nanoFramework.CoreLibrary.NoReflection` |
+🧰 Tools
🪛 LanguageTool
[grammar] ~93-~93: Ensure spelling is correct
Context: ...ses a locally built nanoCLR
Key runsettings config:
- `IsRealHardware=False` — run...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
</details>
<details>
<summary>🪛 markdownlint-cli2 (0.22.0)</summary>
[warning] 9-9: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
---
[warning] 15-15: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
---
[warning] 16-16: Tables should be surrounded by blank lines
(MD058, blanks-around-tables)
---
[warning] 23-23: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
---
[warning] 28-28: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
---
[warning] 62-62: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
---
[warning] 67-67: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
---
[warning] 68-68: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
---
[warning] 79-79: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
---
[warning] 99-99: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
---
[warning] 108-108: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
---
[warning] 110-110: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
---
[warning] 115-115: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
---
[warning] 116-116: Tables should be surrounded by blank lines
(MD058, blanks-around-tables)
---
[warning] 123-123: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
---
[warning] 131-131: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
---
[warning] 137-137: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
</details>
</details>
<details>
<summary>🤖 Prompt for AI Agents</summary>
Verify each finding against the current code and only fix it if needed.
In @.github/copilot-instructions.md around lines 9 - 137, The Markdown has
missing blank lines around headings, tables, and fenced code blocks
(MD022/MD058/MD031); fix by ensuring a single blank line before and after each
heading like "### This Is NOT Standard .NET", around the table under "Two
Library Flavours", and before/after fenced code blocks (the three
triple-backtick blocks), and normalize spacing so every heading, table, and
fenced block is separated by exactly one blank line from surrounding paragraphs
or lists throughout the file; scan for repeated occurrences and apply the same
pattern consistently.
</details>
<!-- fingerprinting:phantom:triton:hawk:c2b10d03-38a3-4613-b698-96257449861a -->
<!-- This is an auto-generated comment by CodeRabbit -->
Comment on lines
+28
to
+58
| ``` | ||
| nanoFramework.CoreLibrary/ # Main BCL project (with reflection) | ||
| System/ # All System.* source files | ||
| Collections/ # IEnumerable, ArrayList, generic interfaces | ||
| Diagnostics/ # Debug, Debugger, attributes | ||
| Globalization/ # CultureInfo, DateTimeFormatInfo, etc. | ||
| IO/ # IOException | ||
| Reflection/ # Assembly, MethodInfo, FieldInfo, etc. | ||
| Runtime/ # CompilerServices, InteropServices, Remoting | ||
| Threading/ # Thread, Monitor, Timer, WaitHandle, etc. | ||
| CoreLibrary.nfproj | ||
| Directory.Build.props | ||
|
|
||
| nanoFramework.CoreLibrary.NoReflection/ # BCL without reflection | ||
| System/ # Subset of System.* files (no Reflection/) | ||
| CoreLibrary.NoReflection.nfproj | ||
|
|
||
| Tests/ # Unit tests (one folder per test suite) | ||
| NFUnitTestArithmetic/ | ||
| NFUnitTestArray/ | ||
| NFUnitTestBitConverter/ | ||
| NFUnitTestSystemLib/ # Covers most primitive types, strings, DateTime, etc. | ||
| NFUnitTestThread/ | ||
| ... (21 test suites total) | ||
|
|
||
| nanoFramework.TestFramework/ # Git submodule: test framework source | ||
| azure-pipelines.yml # CI/CD pipeline (Azure Pipelines, Windows) | ||
| nanoFramework.CoreLibrary.sln # Main solution (library + tests) | ||
| nanoFramework.CoreLibrary.Benchmarks.sln # Benchmarks solution | ||
| version.json # Nerdbank.GitVersioning configuration | ||
| ``` |
There was a problem hiding this comment.
Add a language identifier to the repository-structure fenced block.
The fenced block starting at Line 28 has no language tag (MD040). Use text for directory trees.
Proposed fix
-```
+```text
nanoFramework.CoreLibrary/ # Main BCL project (with reflection)
System/ # All System.* source files
...
version.json # Nerdbank.GitVersioning configuration</details>
<!-- suggestion_start -->
<details>
<summary>📝 Committable suggestion</summary>
> ‼️ **IMPORTANT**
> Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
```suggestion
🧰 Tools
🪛 markdownlint-cli2 (0.22.0)
[warning] 28-28: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/copilot-instructions.md around lines 28 - 58, The Markdown fenced
code block containing the repository tree (starting with the line
"nanoFramework.CoreLibrary/ # Main BCL project (with reflection)") is
missing a language tag and triggers MD040; fix it by adding the language
identifier `text` immediately after the opening triple backticks (i.e., replace
the opening "```" before the "nanoFramework.CoreLibrary/" block with "```text")
so the directory tree is properly tagged.
Comment on lines
+67
to
+69
| displayName: Flag availability of local nanoCLR | ||
| name: NANOCLR_CHECK | ||
| inputs: |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Verify producer step name and output variable declaration
rg -n -C2 "name:\s*NANOCLR_CHECK|HAS_LOCAL_NANOCLR|isOutput=true" azure-pipelines-templates/check-nf-interpreter-to-test.yml
# Verify consumer dependency output key and env mapping
rg -n -C3 "dependencies\\['check-nf-interpreter-to-test'\\]\\.outputs|HAS_LOCAL_NANOCLR|NANOCLR_CHECK|Flag_availability_of_local_nanoCLR" azure-pipelines.ymlRepository: nanoframework/CoreLibrary
Length of output: 1123
Fix output-variable reference: use step name instead of displayName.
The step named NANOCLR_CHECK (line 68) declares output variable HAS_LOCAL_NANOCLR (line 72), but line 86 in azure-pipelines.yml references it using the displayName Flag_availability_of_local_nanoCLR instead of the step name. In Azure Pipelines, output references require the step's name field. Additionally, line 79 reads $env:NANOCLR_CHECK_HAS_LOCAL_NANOCLR, which won't exist—the env mapping should produce $env:HAS_LOCAL_NANOCLR or be corrected to match the reference. Fix the output reference path and/or the variable name in the subsequent step.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@azure-pipelines-templates/check-nf-interpreter-to-test.yml` around lines 67 -
69, The output reference and environment variable are using the displayName
instead of the step name and the wrong env var name; update any references that
point to Flag_availability_of_local_nanoCLR to use the step name NANOCLR_CHECK
(e.g., reference the step outputs via NANOCLR_CHECK instead of the displayName)
and fix the env mapping so the produced env var is HAS_LOCAL_NANOCLR (or change
subsequent uses from $env:NANOCLR_CHECK_HAS_LOCAL_NANOCLR to
$env:HAS_LOCAL_NANOCLR) so the output and env variable names match the step name
and declared output (NANOCLR_CHECK and HAS_LOCAL_NANOCLR).
Comment on lines
+74
to
+86
| - task: PowerShell@2 | ||
| displayName: Set unit test runsettings path | ||
| inputs: | ||
| targetType: 'inline' | ||
| script: | | ||
| $hasLocalNanoClr = $env:NANOCLR_CHECK_HAS_LOCAL_NANOCLR | ||
| if ($hasLocalNanoClr -eq 'true') { | ||
| Write-Host "##vso[task.setvariable variable=UNIT_TEST_RUNSETTINGS]$(System.DefaultWorkingDirectory)\local_clr.runsettings" | ||
| } else { | ||
| Write-Host "##vso[task.setvariable variable=UNIT_TEST_RUNSETTINGS]$(System.DefaultWorkingDirectory)\.runsettings" | ||
| } | ||
| env: | ||
| HAS_LOCAL_NANOCLR: $[ dependencies['check-nf-interpreter-to-test'].outputs['Flag_availability_of_local_nanoCLR.HAS_LOCAL_NANOCLR'] ] |
There was a problem hiding this comment.
Read the environment variable you actually inject.
This step maps HAS_LOCAL_NANOCLR in env:, but the script reads NANOCLR_CHECK_HAS_LOCAL_NANOCLR. As written, the explicit mapping at Line 85-Line 86 never affects UNIT_TEST_RUNSETTINGS.
🛠️ Minimal fix
- $hasLocalNanoClr = $env:NANOCLR_CHECK_HAS_LOCAL_NANOCLR
+ $hasLocalNanoClr = $env:HAS_LOCAL_NANOCLR🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@azure-pipelines.yml` around lines 74 - 86, The script reads the wrong env var
name; change the PowerShell script to read $env:HAS_LOCAL_NANOCLR (the variable
you inject) instead of $env:NANOCLR_CHECK_HAS_LOCAL_NANOCLR so the condition
that sets UNIT_TEST_RUNSETTINGS actually uses the mapped value; update the check
in that inline script where HAS_LOCAL_NANOCLR/NANOCLR_CHECK_HAS_LOCAL_NANOCLR
are referenced to use HAS_LOCAL_NANOCLR consistently.
| <nanoFrameworkAdapter> | ||
| <Logging>Verbose</Logging> | ||
| <IsRealHardware>False</IsRealHardware> | ||
| <PathToLocalCLRInstance>D:\a\_temp\nanoclr\nanoFramework.nanoCLR.dll</PathToLocalCLRInstance> |
There was a problem hiding this comment.
Avoid hard-coded agent path for local CLR.
Line 14 pins PathToLocalCLRInstance to D:\a\_temp\..., which is agent-specific and can fail on different hosted/self-hosted layouts.
Use a runtime-resolved path strategy (for example, pipeline token replacement into this file) so it always matches the actual artifact download directory.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@local_clr.runsettings` at line 14, The runsettings file currently hard-codes
PathToLocalCLRInstance to an agent-specific location; change this to a
runtime-resolved value so it works across agents. Replace the static
PathToLocalCLRInstance entry in local_clr.runsettings with a token/placeholder
(e.g., %(NANOCLR_PATH) or ${NANOCLR_PATH}) and update the pipeline/job that
downloads the artifact to set that token to the actual download directory (or
write the resolved path into the file as a pipeline step) so
PathToLocalCLRInstance is populated at runtime rather than being fixed to
D:\a\_temp\...\nanoFramework.nanoCLR.dll.
Comment on lines
49
to
52
| public override bool Equals(object obj) | ||
| { | ||
| var s = obj as String; | ||
| var s = obj as string; | ||
| return s != null && Equals(this, s); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
rg -n '\bvar\b' nanoFramework.CoreLibrary/System/String.csRepository: nanoframework/CoreLibrary
Length of output: 106
Use an explicit type instead of var on line 51.
The coding guidelines require explicit types in C# code.
Suggested fix
- var s = obj as string;
+ string s = obj as string;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| public override bool Equals(object obj) | |
| { | |
| var s = obj as String; | |
| var s = obj as string; | |
| return s != null && Equals(this, s); | |
| public override bool Equals(object obj) | |
| { | |
| string s = obj as string; | |
| return s != null && Equals(this, s); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@nanoFramework.CoreLibrary/System/String.cs` around lines 49 - 52, In
String.Equals(object obj) replace the implicitly typed local 'var s = obj as
string;' with an explicit type declaration 'string s = obj as string;' so the
Equals method uses an explicitly typed local; update the line in the Equals
method (the local named s) accordingly.
Comment on lines
+61
to
+70
| #if NANOCLR_REFLECTION | ||
|
|
||
| #nullable enable | ||
| [MethodImpl(MethodImplOptions.InternalCall)] | ||
| public static extern bool Equals(String a, String b); | ||
| public static extern bool Equals(string a, string b); | ||
| #nullable restore | ||
|
|
||
| #else | ||
| [MethodImpl(MethodImplOptions.InternalCall)] | ||
| public static extern bool Equals(string a, string b); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
rg -n 'DefineConstants|NANOCLR_REFLECTION' nanoFramework.CoreLibrary/CoreLibrary.nfproj
rg -n '#if NANOCLR_REFLECTION|public static extern bool Equals\(string|public static extern bool operator ==\(|public static extern bool operator !=\(' nanoFramework.CoreLibrary/System/String.csRepository: nanoframework/CoreLibrary
Length of output: 988
Make Equals(string, string) parameters nullable to match the == and != operators.
The NANOCLR_REFLECTION branch (defined in CoreLibrary.nfproj line 36) is the active configuration. Within this branch, Equals(string a, string b) has non-nullable parameters while the sibling operators ==(string? a, string? b) and !=(string? a, string? b) accept nullable inputs. This inconsistency on the public surface forces callers to suppress warnings when passing null to a method that silently accepts it.
Suggested fix
- public static extern bool Equals(string a, string b);
+ public static extern bool Equals(string? a, string? b);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@nanoFramework.CoreLibrary/System/String.cs` around lines 61 - 70, In the
NANOCLR_REFLECTION branch update the signature of the extern method Equals so it
accepts nullable inputs—change public static extern bool Equals(string a, string
b) to use nullable parameters (string? a, string? b) to match the public
operators ==(string? a, string? b) and !=(string? a, string? b); locate the
method definition in System/String.cs inside the `#if` NANOCLR_REFLECTION block
and adjust the parameter types accordingly so callers can pass null without
warnings.
Comment on lines
+15
to
+137
| var empty = Guid.Empty; | ||
| var notEmpty = Guid.NewGuid(); | ||
| Assert.IsFalse(empty == notEmpty); | ||
| } | ||
|
|
||
| [TestMethod] | ||
| public void Guid_Empty_IsAllZeros() | ||
| { | ||
| var empty = Guid.Empty; | ||
| var bytes = empty.ToByteArray(); | ||
| foreach (var b in bytes) | ||
| { | ||
| Assert.AreEqual((byte)0, b); | ||
| } | ||
| } | ||
|
|
||
| [TestMethod] | ||
| public void Guid_Constructor_ByteArray_RoundTrip() | ||
| { | ||
| var original = Guid.NewGuid(); | ||
| var bytes = original.ToByteArray(); | ||
| var roundTrip = new Guid(bytes); | ||
| Assert.AreEqual(original, roundTrip); | ||
| } | ||
|
|
||
| [TestMethod] | ||
| public void Guid_Equals_And_Operator() | ||
| { | ||
| var g1 = Guid.NewGuid(); | ||
| var g2 = new Guid(g1.ToByteArray()); | ||
| Assert.IsTrue(g1.Equals(g2)); | ||
| Assert.IsTrue(g1 == g2); | ||
| Assert.IsFalse(g1 != g2); | ||
| } | ||
|
|
||
| [TestMethod] | ||
| public void Guid_NotEquals_And_Operator() | ||
| { | ||
| var g1 = Guid.NewGuid(); | ||
| var g2 = Guid.NewGuid(); | ||
| Assert.IsFalse(g1.Equals(g2)); | ||
| Assert.IsFalse(g1 == g2); | ||
| Assert.IsTrue(g1 != g2); | ||
| } | ||
|
|
||
| [TestMethod] | ||
| public void Guid_ToString_And_Parse() | ||
| { | ||
| var g1 = Guid.NewGuid(); | ||
| var str = g1.ToString(); | ||
| var g2 = new Guid(str); | ||
| Assert.AreEqual(g1, g2); | ||
| } | ||
|
|
||
| [TestMethod] | ||
| public void Guid_GetHashCode_Consistent() | ||
| { | ||
| var g1 = Guid.NewGuid(); | ||
| var g2 = new Guid(g1.ToByteArray()); | ||
| Assert.AreEqual(g1.GetHashCode(), g2.GetHashCode()); | ||
| } | ||
|
|
||
| [TestMethod] | ||
| public void Guid_CompareTo_Object_And_Self() | ||
| { | ||
| var g1 = Guid.NewGuid(); | ||
| var g2 = new Guid(g1.ToByteArray()); | ||
| Assert.AreEqual(0, g1.CompareTo(g2)); | ||
| Assert.AreEqual(0, g1.CompareTo((object)g2)); | ||
| Assert.AreEqual(1, g1.CompareTo(null)); | ||
| } | ||
|
|
||
| [TestMethod] | ||
| public void Guid_CompareTo_InvalidType_Throws() | ||
| { | ||
| var g1 = Guid.NewGuid(); | ||
| Assert.ThrowsException(typeof(ArgumentException), () => | ||
| { | ||
| g1.CompareTo("not a guid"); | ||
| }); | ||
| } | ||
|
|
||
| [TestMethod] | ||
| public void Guid_TryParseGuidWithDashes_Valid() | ||
| { | ||
| var g1 = Guid.NewGuid(); | ||
| var str = g1.ToString(); | ||
| bool parsed = Guid.TryParse(str, out var g2); | ||
| Assert.IsTrue(parsed); | ||
| Assert.AreEqual(g1, g2); | ||
| } | ||
|
|
||
| [TestMethod] | ||
| public void Guid_TryParseGuidWithDashes_Invalid() | ||
| { | ||
| bool parsed = Guid.TryParse("invalid-guid", out var g2); | ||
| Assert.IsFalse(parsed); | ||
| Assert.AreEqual(Guid.Empty, g2); | ||
| } | ||
|
|
||
| [TestMethod] | ||
| public void Guid_Constructor_String_WithBraces() | ||
| { | ||
| var g1 = Guid.NewGuid(); | ||
| var str = "{" + g1.ToString() + "}"; | ||
| var g2 = new Guid(str); | ||
| Assert.AreEqual(g1, g2); | ||
| } | ||
|
|
||
| [TestMethod] | ||
| public void Guid_Constructor_String_Invalid_Throws() | ||
| { | ||
| Assert.ThrowsException(typeof(ArgumentException), () => | ||
| { | ||
| var g = new Guid("invalid-guid"); | ||
| }); | ||
| } | ||
|
|
||
| [TestMethod] | ||
| public void Guid_GetHashCode_Empty() | ||
| { | ||
| var empty = Guid.Empty; | ||
| Assert.AreEqual(0, empty.GetHashCode()); |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Use explicit local types in this test file.
The new tests consistently use var, which is against the repo's C# style.
As per coding guidelines, "Do not use 'var' and maintain explicit types in code to match the nanoFramework coding standards."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Tests/NFUnitTestTypes/UnitTestGuid.cs` around lines 15 - 137, Replace all
implicit var declarations in this test file with explicit types to follow the
repo C# style: e.g. in methods Guid_Empty_IsAllZeros,
Guid_Constructor_ByteArray_RoundTrip, Guid_Equals_And_Operator,
Guid_NotEquals_And_Operator, Guid_ToString_And_Parse,
Guid_GetHashCode_Consistent, Guid_CompareTo_Object_And_Self,
Guid_TryParseGuidWithDashes_Valid, Guid_TryParseGuidWithDashes_Invalid,
Guid_Constructor_String_WithBraces, Guid_Constructor_String_Invalid_Throws and
Guid_GetHashCode_Empty change usages like "var empty", "var bytes", "var
original", "var roundTrip", "var g1", "var g2", "var str", "var parsed" to their
explicit types (Guid, byte[], bool, string, etc.); update all out variables
(e.g. Guid.TryParse(..., out var g2)) to use explicit type declarations or
declare the variable on a separate line before the call so no var remains.
Comment on lines
+77
to
+138
| [TestMethod] | ||
| public void Guid_CompareTo_Object_And_Self() | ||
| { | ||
| var g1 = Guid.NewGuid(); | ||
| var g2 = new Guid(g1.ToByteArray()); | ||
| Assert.AreEqual(0, g1.CompareTo(g2)); | ||
| Assert.AreEqual(0, g1.CompareTo((object)g2)); | ||
| Assert.AreEqual(1, g1.CompareTo(null)); | ||
| } | ||
|
|
||
| [TestMethod] | ||
| public void Guid_CompareTo_InvalidType_Throws() | ||
| { | ||
| var g1 = Guid.NewGuid(); | ||
| Assert.ThrowsException(typeof(ArgumentException), () => | ||
| { | ||
| g1.CompareTo("not a guid"); | ||
| }); | ||
| } | ||
|
|
||
| [TestMethod] | ||
| public void Guid_TryParseGuidWithDashes_Valid() | ||
| { | ||
| var g1 = Guid.NewGuid(); | ||
| var str = g1.ToString(); | ||
| bool parsed = Guid.TryParse(str, out var g2); | ||
| Assert.IsTrue(parsed); | ||
| Assert.AreEqual(g1, g2); | ||
| } | ||
|
|
||
| [TestMethod] | ||
| public void Guid_TryParseGuidWithDashes_Invalid() | ||
| { | ||
| bool parsed = Guid.TryParse("invalid-guid", out var g2); | ||
| Assert.IsFalse(parsed); | ||
| Assert.AreEqual(Guid.Empty, g2); | ||
| } | ||
|
|
||
| [TestMethod] | ||
| public void Guid_Constructor_String_WithBraces() | ||
| { | ||
| var g1 = Guid.NewGuid(); | ||
| var str = "{" + g1.ToString() + "}"; | ||
| var g2 = new Guid(str); | ||
| Assert.AreEqual(g1, g2); | ||
| } | ||
|
|
||
| [TestMethod] | ||
| public void Guid_Constructor_String_Invalid_Throws() | ||
| { | ||
| Assert.ThrowsException(typeof(ArgumentException), () => | ||
| { | ||
| var g = new Guid("invalid-guid"); | ||
| }); | ||
| } | ||
|
|
||
| [TestMethod] | ||
| public void Guid_GetHashCode_Empty() | ||
| { | ||
| var empty = Guid.Empty; | ||
| Assert.AreEqual(0, empty.GetHashCode()); | ||
| } |
There was a problem hiding this comment.
Add coverage for the non-happy-path Guid cases introduced here.
These tests stop at equality/null CompareTo and one happy-path / one invalid-path TryParse. They don't exercise non-equal ordering, null/empty parse input, or any additional parse formats the API promises, so the current Guid regressions stay undetected.
Based on learnings, "Every new API must have test coverage for all methods, properties, events, and exceptions."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Tests/NFUnitTestTypes/UnitTestGuid.cs` around lines 77 - 138, Add tests to
cover non-happy-path ordering and additional parsing formats: create new tests
that assert Guid_CompareTo_Order where two different GUIDs produce non-zero
CompareTo results in both directions (e.g., verify g1.CompareTo(g2) is <0 and
g2.CompareTo(g1) is >0) and that CompareTo((object)null) returns 1 (reference
Guid_CompareTo_Object_And_Self). Add TryParse edge-case tests:
Guid_TryParse_NullOrEmpty_Invalid which calls Guid.TryParse(null, out var) and
Guid.TryParse("", out var) and asserts false and Guid.Empty, and
Guid_TryParse_AlternateFormats_Valid to TryParse the same GUID formatted as "N",
"B", and "P" strings and assert success and equality (to complement
Guid_TryParseGuidWithDashes_Valid). Also add a
Guid_ParseOrCtor_ParenthesesAndNoDashes test to construct/parse strings with
parentheses and the "N" format and assert equality, and a
Guid_Parse_Invalid_Empty_Throws or equivalent to confirm Parse/constructor
behavior for empty string if applicable; update or add tests named above
(Guid_CompareTo_Order, Guid_TryParse_NullOrEmpty_Invalid,
Guid_TryParse_AlternateFormats_Valid, Guid_ParseOrCtor_ParenthesesAndNoDashes)
accordingly.
Comment on lines
+699
to
+986
| [TestMethod] | ||
| public void EqualityOperatorTests() | ||
| { | ||
| byte[] array1 = new byte[4] { 0x01, 0x02, 0x03, 0x04 }; | ||
| byte[] array2 = new byte[4] { 0x01, 0x02, 0x03, 0x04 }; | ||
| byte[] array3 = new byte[4] { 0x01, 0x02, 0x03, 0x05 }; | ||
|
|
||
| ReadOnlySpan<byte> span1 = new ReadOnlySpan<byte>(array1); | ||
| ReadOnlySpan<byte> span2 = new ReadOnlySpan<byte>(array2); | ||
| ReadOnlySpan<byte> span3 = new ReadOnlySpan<byte>(array3); | ||
|
|
||
| // Test equality with same content (content-based equality) | ||
| Assert.IsTrue(span1 == span2, "ReadOnlySpans with same content should be equal"); | ||
| Assert.IsFalse(span1 != span2, "ReadOnlySpans with same content should not be not-equal"); | ||
|
|
||
| // Test inequality with different content | ||
| Assert.IsTrue(span1 != span3, "ReadOnlySpans with different content should not be equal"); | ||
| Assert.IsFalse(span1 == span3, "ReadOnlySpans with different content should not be equal"); | ||
|
|
||
| // Test empty spans | ||
| ReadOnlySpan<byte> empty1 = new ReadOnlySpan<byte>(); | ||
| ReadOnlySpan<byte> empty2 = new ReadOnlySpan<byte>(); | ||
|
|
||
| Assert.IsTrue(empty1 == empty2, "Empty ReadOnlySpans should be equal"); | ||
| Assert.IsFalse(empty1 != empty2, "Empty ReadOnlySpans should not be not-equal"); | ||
| } | ||
|
|
||
| [TestMethod] | ||
| public void EqualityWithPartialSpansTests() | ||
| { | ||
| byte[] array1 = new byte[10] { 0, 1, 2, 3, 4, 5, 6, 7, 8, 9 }; | ||
| byte[] array2 = new byte[10] { 0, 1, 2, 3, 4, 5, 6, 7, 8, 9 }; | ||
|
|
||
| // Create two spans from different arrays with same content | ||
| ReadOnlySpan<byte> span1 = new ReadOnlySpan<byte>(array1, 2, 5); | ||
| ReadOnlySpan<byte> span2 = new ReadOnlySpan<byte>(array2, 2, 5); | ||
|
|
||
| // They should be equal (content-based equality) | ||
| Assert.IsTrue(span1 == span2, "Partial spans with same content should be equal"); | ||
| Assert.IsFalse(span1 != span2, "Partial spans with same content should not be not-equal"); | ||
|
|
||
| // Create span from different array with different content | ||
| byte[] array3 = new byte[10] { 0, 1, 2, 3, 99, 5, 6, 7, 8, 9 }; | ||
| ReadOnlySpan<byte> span3 = new ReadOnlySpan<byte>(array3, 2, 5); | ||
| Assert.IsFalse(span1 == span3, "Partial spans with different content should not be equal"); | ||
| Assert.IsTrue(span1 != span3, "Partial spans with different content should be not-equal"); | ||
|
|
||
| // Create span with different length | ||
| ReadOnlySpan<byte> span4 = new ReadOnlySpan<byte>(array1, 2, 4); | ||
| Assert.IsFalse(span1 == span4, "Partial spans with different length should not be equal"); | ||
| Assert.IsTrue(span1 != span4, "Partial spans with different length should be not-equal"); | ||
| } | ||
|
|
||
| [TestMethod] | ||
| public void EqualityWithSlicedSpansTests() | ||
| { | ||
| byte[] array1 = new byte[10] { 0, 1, 2, 3, 4, 5, 6, 7, 8, 9 }; | ||
| byte[] array2 = new byte[10] { 0, 1, 2, 3, 4, 5, 6, 7, 8, 9 }; | ||
|
|
||
| ReadOnlySpan<byte> originalSpan1 = new ReadOnlySpan<byte>(array1); | ||
| ReadOnlySpan<byte> originalSpan2 = new ReadOnlySpan<byte>(array2); | ||
|
|
||
| // Create two slices from different arrays with same content | ||
| ReadOnlySpan<byte> slice1 = originalSpan1.Slice(2, 5); | ||
| ReadOnlySpan<byte> slice2 = originalSpan2.Slice(2, 5); | ||
|
|
||
| // They should be equal (content-based equality) | ||
| Assert.IsTrue(slice1 == slice2, "Slices with same content should be equal"); | ||
| Assert.IsFalse(slice1 != slice2, "Slices with same content should not be not-equal"); | ||
|
|
||
| // Create a slice with different content | ||
| byte[] array3 = new byte[10] { 0, 1, 2, 99, 4, 5, 6, 7, 8, 9 }; | ||
| ReadOnlySpan<byte> originalSpan3 = new ReadOnlySpan<byte>(array3); | ||
| ReadOnlySpan<byte> slice3 = originalSpan3.Slice(2, 5); | ||
| Assert.IsFalse(slice1 == slice3, "Slices with different content should not be equal"); | ||
|
|
||
| // Create a slice with different length | ||
| ReadOnlySpan<byte> slice4 = originalSpan1.Slice(2, 4); | ||
| Assert.IsFalse(slice1 == slice4, "Slices with different length should not be equal"); | ||
| } | ||
|
|
||
| [TestMethod] | ||
| public void EqualityWithNullArrayTests() | ||
| { | ||
| // Test equality with null-backed spans | ||
| ReadOnlySpan<byte> nullSpan1 = new ReadOnlySpan<byte>(null); | ||
| ReadOnlySpan<byte> nullSpan2 = new ReadOnlySpan<byte>(null); | ||
| ReadOnlySpan<byte> nullSpan3 = new ReadOnlySpan<byte>(null, 0, 0); | ||
|
|
||
| // All null-backed spans should be equal (both are empty) | ||
| Assert.IsTrue(nullSpan1 == nullSpan2, "Null-backed spans should be equal"); | ||
| Assert.IsTrue(nullSpan1 == nullSpan3, "Null-backed spans with explicit 0,0 should be equal"); | ||
| Assert.IsFalse(nullSpan1 != nullSpan2, "Null-backed spans should not be not-equal"); | ||
|
|
||
| // Compare with Empty | ||
| ReadOnlySpan<byte> emptySpan = ReadOnlySpan<byte>.Empty; | ||
| Assert.IsTrue(nullSpan1 == emptySpan, "Null-backed span should equal Empty span"); | ||
| } | ||
|
|
||
| [TestMethod] | ||
| public void EqualityContentBasedTests() | ||
| { | ||
| // Create two arrays with identical content but different references | ||
| byte[] array1 = new byte[5] { 10, 20, 30, 40, 50 }; | ||
| byte[] array2 = new byte[5] { 10, 20, 30, 40, 50 }; | ||
|
|
||
| ReadOnlySpan<byte> span1 = new ReadOnlySpan<byte>(array1); | ||
| ReadOnlySpan<byte> span2 = new ReadOnlySpan<byte>(array2); | ||
|
|
||
| // ReadOnlySpans with same content should be equal (content-based equality) | ||
| Assert.IsTrue(span1 == span2, "ReadOnlySpans with same content should be equal"); | ||
| Assert.IsFalse(span1 != span2, "ReadOnlySpans with same content should not be not-equal"); | ||
|
|
||
| // Now create another span from array1 | ||
| ReadOnlySpan<byte> span1Copy = new ReadOnlySpan<byte>(array1); | ||
| Assert.IsTrue(span1 == span1Copy, "ReadOnlySpans from same array should be equal"); | ||
|
|
||
| // Modify array1 content | ||
| array1[2] = 99; | ||
|
|
||
| // Spans should no longer be equal because content changed | ||
| Assert.IsFalse(span1 == span2, "ReadOnlySpans should not be equal after content changes"); | ||
| Assert.IsTrue(span1 != span2, "ReadOnlySpans should be not-equal after content changes"); | ||
|
|
||
| // But span1 and span1Copy still reference the same array so should see same content | ||
| Assert.IsTrue(span1 == span1Copy, "ReadOnlySpans from same array should remain equal"); | ||
| } | ||
|
|
||
| [TestMethod] | ||
| public void EqualityWithStackAllocatedSpansTests() | ||
| { | ||
| // Stack-allocated spans with same content should be equal | ||
| ReadOnlySpan<byte> stackSpan1 = stackalloc byte[5] { 1, 2, 3, 4, 5 }; | ||
| ReadOnlySpan<byte> stackSpan2 = stackalloc byte[5] { 1, 2, 3, 4, 5 }; | ||
|
|
||
| // Stack-allocated spans with identical content should be equal (content-based) | ||
| Assert.IsTrue(stackSpan1 == stackSpan2, "Stack-allocated spans with same content should be equal"); | ||
| Assert.IsFalse(stackSpan1 != stackSpan2, "Stack-allocated spans with same content should not be not-equal"); | ||
|
|
||
| // Stack-allocated spans with different content should not be equal | ||
| ReadOnlySpan<byte> stackSpan3 = stackalloc byte[5] { 1, 2, 99, 4, 5 }; | ||
| Assert.IsFalse(stackSpan1 == stackSpan3, "Stack-allocated spans with different content should not be equal"); | ||
| Assert.IsTrue(stackSpan1 != stackSpan3, "Stack-allocated spans with different content should be not-equal"); | ||
|
|
||
| // Create a "copy" reference to the same stack span | ||
| ReadOnlySpan<byte> stackSpan1Copy = stackSpan1; | ||
| Assert.IsTrue(stackSpan1 == stackSpan1Copy, "Same stack-allocated span should equal itself"); | ||
| } | ||
|
|
||
| [TestMethod] | ||
| public void EqualityTransitivityTests() | ||
| { | ||
| byte[] array1 = new byte[5] { 1, 2, 3, 4, 5 }; | ||
| byte[] array2 = new byte[5] { 1, 2, 3, 4, 5 }; | ||
| byte[] array3 = new byte[5] { 1, 2, 3, 4, 5 }; | ||
|
|
||
| ReadOnlySpan<byte> span1 = new ReadOnlySpan<byte>(array1); | ||
| ReadOnlySpan<byte> span2 = new ReadOnlySpan<byte>(array2); | ||
| ReadOnlySpan<byte> span3 = new ReadOnlySpan<byte>(array3); | ||
|
|
||
| // Test transitivity: if span1 == span2 and span2 == span3, then span1 == span3 | ||
| Assert.IsTrue(span1 == span2, "span1 should equal span2"); | ||
| Assert.IsTrue(span2 == span3, "span2 should equal span3"); | ||
| Assert.IsTrue(span1 == span3, "span1 should equal span3 (transitivity)"); | ||
| } | ||
|
|
||
| [TestMethod] | ||
| public void EqualityReflexivityTests() | ||
| { | ||
| byte[] array = new byte[5] { 1, 2, 3, 4, 5 }; | ||
| ReadOnlySpan<byte> span = new ReadOnlySpan<byte>(array); | ||
|
|
||
| // Test reflexivity: span should equal itself | ||
| Assert.IsTrue(span == span, "ReadOnlySpan should equal itself (reflexivity)"); | ||
| Assert.IsFalse(span != span, "ReadOnlySpan should not be not-equal to itself"); | ||
|
|
||
| // Test with empty span | ||
| ReadOnlySpan<byte> emptySpan = ReadOnlySpan<byte>.Empty; | ||
| Assert.IsTrue(emptySpan == emptySpan, "Empty ReadOnlySpan should equal itself"); | ||
| } | ||
|
|
||
| [TestMethod] | ||
| public void EqualitySymmetryTests() | ||
| { | ||
| byte[] array1 = new byte[5] { 1, 2, 3, 4, 5 }; | ||
| byte[] array2 = new byte[5] { 1, 2, 3, 4, 5 }; | ||
| byte[] array3 = new byte[5] { 1, 2, 99, 4, 5 }; | ||
|
|
||
| ReadOnlySpan<byte> span1 = new ReadOnlySpan<byte>(array1); | ||
| ReadOnlySpan<byte> span2 = new ReadOnlySpan<byte>(array2); | ||
| ReadOnlySpan<byte> span3 = new ReadOnlySpan<byte>(array3); | ||
|
|
||
| // Test symmetry: if span1 == span2, then span2 == span1 | ||
| Assert.IsTrue(span1 == span2, "span1 should equal span2"); | ||
| Assert.IsTrue(span2 == span1, "span2 should equal span1 (symmetry)"); | ||
|
|
||
| // Test symmetry with inequality | ||
| Assert.IsFalse(span1 == span3, "span1 should not equal span3"); | ||
| Assert.IsFalse(span3 == span1, "span3 should not equal span1 (symmetry)"); | ||
| } | ||
|
|
||
| [TestMethod] | ||
| public void EqualityWithDifferentOffsetsTests() | ||
| { | ||
| byte[] array = new byte[10] { 0, 1, 2, 3, 4, 5, 6, 7, 8, 9 }; | ||
|
|
||
| // Create spans with overlapping but different regions | ||
| ReadOnlySpan<byte> span1 = new ReadOnlySpan<byte>(array, 2, 5); // [2,3,4,5,6] | ||
| ReadOnlySpan<byte> span2 = new ReadOnlySpan<byte>(array, 3, 5); // [3,4,5,6,7] | ||
|
|
||
| // They have different content so should not be equal | ||
| Assert.IsFalse(span1 == span2, "ReadOnlySpans with different content should not be equal"); | ||
| Assert.IsTrue(span1 != span2, "ReadOnlySpans with different content should be not-equal"); | ||
|
|
||
| // Create span with same content from different array | ||
| byte[] array2 = new byte[10] { 0, 1, 2, 3, 4, 5, 6, 7, 8, 9 }; | ||
| ReadOnlySpan<byte> span3 = new ReadOnlySpan<byte>(array2, 2, 5); | ||
| Assert.IsTrue(span1 == span3, "ReadOnlySpans with same content should be equal"); | ||
| } | ||
|
|
||
| [TestMethod] | ||
| public void EqualityZeroLengthSpansTests() | ||
| { | ||
| byte[] array1 = new byte[10]; | ||
| byte[] array2 = new byte[10]; | ||
|
|
||
| // Create zero-length spans from different arrays | ||
| ReadOnlySpan<byte> zeroSpan1 = new ReadOnlySpan<byte>(array1, 5, 0); | ||
| ReadOnlySpan<byte> zeroSpan2 = new ReadOnlySpan<byte>(array2, 3, 0); | ||
|
|
||
| // Zero-length spans should be equal (both empty, content-based) | ||
| Assert.IsTrue(zeroSpan1 == zeroSpan2, "Zero-length spans should be equal"); | ||
| Assert.IsFalse(zeroSpan1 != zeroSpan2, "Zero-length spans should not be not-equal"); | ||
|
|
||
| // Zero-length span from same array with different offset | ||
| ReadOnlySpan<byte> zeroSpan3 = new ReadOnlySpan<byte>(array1, 7, 0); | ||
| Assert.IsTrue(zeroSpan1 == zeroSpan3, "Zero-length spans should be equal regardless of offset"); | ||
|
|
||
| // Compare with Empty | ||
| ReadOnlySpan<byte> emptySpan = ReadOnlySpan<byte>.Empty; | ||
| Assert.IsTrue(zeroSpan1 == emptySpan, "Zero-length span should equal Empty span"); | ||
| } | ||
|
|
||
| [TestMethod] | ||
| public void EqualityDifferentLengthsTests() | ||
| { | ||
| byte[] array1 = new byte[6] { 1, 2, 3, 4, 5, 6 }; | ||
| byte[] array2 = new byte[4] { 1, 2, 3, 4 }; | ||
|
|
||
| ReadOnlySpan<byte> span1 = new ReadOnlySpan<byte>(array1); | ||
| ReadOnlySpan<byte> span2 = new ReadOnlySpan<byte>(array2); | ||
|
|
||
| // Spans with different lengths should not be equal | ||
| Assert.IsFalse(span1 == span2, "ReadOnlySpans with different lengths should not be equal"); | ||
| Assert.IsTrue(span1 != span2, "ReadOnlySpans with different lengths should be not-equal"); | ||
|
|
||
| // Test with partial spans that have same initial content but different lengths | ||
| byte[] array3 = new byte[10] { 1, 2, 3, 4, 5, 6, 7, 8, 9, 10 }; | ||
| ReadOnlySpan<byte> span3 = new ReadOnlySpan<byte>(array3, 0, 4); | ||
| ReadOnlySpan<byte> span4 = new ReadOnlySpan<byte>(array3, 0, 6); | ||
|
|
||
| Assert.IsFalse(span3 == span4, "Partial spans with different lengths should not be equal"); | ||
| } | ||
|
|
||
| [TestMethod] | ||
| public void EqualityPartialContentMatchTests() | ||
| { | ||
| // Test equality when only part of the content matches | ||
| byte[] array1 = new byte[5] { 1, 2, 3, 4, 5 }; | ||
| byte[] array2 = new byte[5] { 1, 2, 99, 4, 5 }; | ||
|
|
||
| ReadOnlySpan<byte> span1 = new ReadOnlySpan<byte>(array1); | ||
| ReadOnlySpan<byte> span2 = new ReadOnlySpan<byte>(array2); | ||
|
|
||
| // Should not be equal (one element differs) | ||
| Assert.IsFalse(span1 == span2, "ReadOnlySpans with partial content match should not be equal"); | ||
| Assert.IsTrue(span1 != span2, "ReadOnlySpans with partial content match should be not-equal"); | ||
|
|
||
| // Test with matching prefix | ||
| ReadOnlySpan<byte> span1Prefix = span1.Slice(0, 2); | ||
| ReadOnlySpan<byte> span2Prefix = span2.Slice(0, 2); | ||
| Assert.IsTrue(span1Prefix == span2Prefix, "ReadOnlySpans with matching prefix should be equal"); | ||
|
|
||
| // Test with matching suffix | ||
| ReadOnlySpan<byte> span1Suffix = span1.Slice(3, 2); | ||
| ReadOnlySpan<byte> span2Suffix = span2.Slice(3, 2); | ||
| Assert.IsTrue(span1Suffix == span2Suffix, "ReadOnlySpans with matching suffix should be equal"); | ||
| } |
There was a problem hiding this comment.
These tests codify content-based ReadOnlySpan<T> equality.
nanoFramework.CoreLibrary/System/ReadOnlySpan.cs Lines 226-253 document equality as “same memory” plus length, and nanoFramework.CoreLibrary/System/Span.cs Lines 261-273 in this PR follows that identity model. The assertions here expect spans from different arrays, different slices, and different stackalloc blocks to compare equal whenever their bytes match, so the suite will preserve the current semantic divergence instead of catching it.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Tests/NFUnitTestTypes/UnitTestsReadOnlySpanByte.cs` around lines 699 - 986,
Tests assume content-based equality but the library implements identity-based
equality in ReadOnlySpan/Span; update the test methods (e.g.,
EqualityOperatorTests, EqualityWithPartialSpansTests,
EqualityWithSlicedSpansTests, EqualityWithStackAllocatedSpansTests,
EqualityContentBasedTests, etc.) to assert content equality using a content
comparer rather than the ==/!= operators — for example replace
Assert.IsTrue(span1 == span2) / Assert.IsFalse(span1 != span2) with
Assert.IsTrue(MemoryExtensions.SequenceEqual(span1, span2) or the equivalent
SequenceEqual extension) and replace inequality checks accordingly; keep using
==/!= only when you intend to assert identity/reflexivity (e.g., span == span)
so the tests match ReadOnlySpan.cs / Span.cs semantics.
|
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
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
Motivation and Context
How Has This Been Tested?
Screenshots
Types of changes
Checklist: