X Tutup
Skip to content

Fix TypeConverter test assertion and unignore test#6484

Closed
brianpursley wants to merge 1 commit intonpgsql:mainfrom
brianpursley:fix-range-test
Closed

Fix TypeConverter test assertion and unignore test#6484
brianpursley wants to merge 1 commit intonpgsql:mainfrom
brianpursley:fix-range-test

Conversation

@brianpursley
Copy link
Copy Markdown
Contributor

Fixed the TypeConverter test assertion to check empty range instead of using NUnit's EmptyConstraint which can't check NpgsqlRange emptiness.

It looks like this was inadvertently changed in the NUnit conversion PR #6183. This PR essentially puts it back to what it was before, just with the newer assertion syntax.

I'm also attempting to unignore this test to see if it passes now on the build server.

… of using NUnit EmptyConstraint which can't handle NpgsqlRange emptiness.
Copilot AI review requested due to automatic review settings March 11, 2026 15:14
Copy link
Copy Markdown

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

Updates a previously ignored NpgsqlRange<T> TypeConverter test to use an assertion that correctly detects NpgsqlRange emptiness under NUnit v4, and re-enables the test to run in CI.

Changes:

  • Un-ignores the TypeConverter test in RangeTests.
  • Replaces Is.Empty with an equality assertion against NpgsqlRange<int>.Empty.
Comments suppressed due to low confidence (1)

test/Npgsql.Tests/Types/RangeTests.cs:381

  • This test mutates global TypeDescriptor state via RangeTypeConverter.Register(), and TypeDescriptor also caches converters. If a converter for NpgsqlRange was already cached (which can differ between local runs and CI / parallel execution), AddAttributes alone won’t update the cached converter and this assertion can still fail intermittently. To make the test reliable when un-ignoring it, refresh the TypeDescriptor cache after registering (e.g., TypeDescriptor.Refresh(typeof(NpgsqlRange))) and consider marking the test (or class) as [NonParallelizable] since it changes global process-wide state.
    [Test]
    public void TypeConverter()
    {
        // Arrange
        NpgsqlRange<int>.RangeTypeConverter.Register();

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@brianpursley
Copy link
Copy Markdown
Contributor Author

Hmm, I see this comment from Copilot that could explain why it might fail on the build server:

This test mutates global TypeDescriptor state via RangeTypeConverter.Register(), and TypeDescriptor also caches converters. If a converter for NpgsqlRange was already cached (which can differ between local runs and CI / parallel execution), AddAttributes alone won’t update the cached converter and this assertion can still fail intermittently. To make the test reliable when un-ignoring it, refresh the TypeDescriptor cache after registering (e.g., TypeDescriptor.Refresh(typeof(NpgsqlRange))) and consider marking the test (or class) as [NonParallelizable] since it changes global process-wide state.

Probably shouldn't un-ignore this test then...

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

X Tutup