Skip to content

Improve String dictionary key strategy tests to verify nested value conversion#1777

Open
ShivaHuang wants to merge 1 commit intoswiftlang:mainfrom
ShivaHuang:improve_JSONEncoderTests
Open

Improve String dictionary key strategy tests to verify nested value conversion#1777
ShivaHuang wants to merge 1 commit intoswiftlang:mainfrom
ShivaHuang:improve_JSONEncoderTests

Conversation

@ShivaHuang
Copy link
Copy Markdown
Contributor

Update both tests to use a nested Codable struct as the dictionary value.

Motivation:

While reviewing #1526, we noticed encodingDictionaryStringKeyConversionUntouched and
decodingDictionaryStringKeyConversionUntouched only verify that String dictionary keys
are preserved, but don't check that key strategies are still correctly applied to nested
struct properties.

Modifications:

  • Update decodingDictionaryStringKeyConversionUntouched to use DecodeMe3 values
  • Update encodingDictionaryStringKeyConversionUntouched to use DecodeMe3 values
  • Verify String dictionary keys bypass strategy while nested properties apply it
  • Ensures consistent comprehensive test coverage for dictionary key behavior

Result:

Tests now verify that dictionary keys bypass the strategy AND nested properties apply it

Testing:

  • encodingDictionaryStringKeyConversionUntouched
  • decodingDictionaryStringKeyConversionUntouched

@jmschonfeld
Copy link
Copy Markdown
Contributor

@swift-ci please test

@itingliu itingliu requested a review from kperryua February 24, 2026 22:45
@ShivaHuang
Copy link
Copy Markdown
Contributor Author

ShivaHuang commented Mar 4, 2026

Hi @kperryua, I believe there was some temperate issue in CI system, can you trigger testing again?

@kperryua
Copy link
Copy Markdown
Contributor

kperryua commented Mar 4, 2026

@swift-ci please test

@ShivaHuang
Copy link
Copy Markdown
Contributor Author

It looks like this failure is a CI infrastructure issue rather than a code problem - the Windows runner failed to connect to the Docker daemon (open //./pipe/docker_engine: The system cannot find the file specified), which is unrelated to the changes in this PR. All other recent PRs passed the same Windows (nightly-main) job without issue.

Could someone look into the CI runner, or alternatively, would merging main into this PR help trigger a fresh runner assignment?

@ShivaHuang ShivaHuang force-pushed the improve_JSONEncoderTests branch from 039aa54 to 4559998 Compare March 24, 2026 02:59
- Update decodingDictionaryStringKeyConversionUntouched to use DecodeMe3 values
- Update encodingDictionaryStringKeyConversionUntouched to use DecodeMe3 values
- Verify String dictionary keys bypass strategy while nested properties apply it
- Ensures consistent comprehensive test coverage for dictionary key behavior
@ShivaHuang ShivaHuang force-pushed the improve_JSONEncoderTests branch from 4559998 to 67e51e1 Compare March 31, 2026 02:39
@ShivaHuang
Copy link
Copy Markdown
Contributor Author

The CI failures are unrelated to this PR's changes:

  • Wasm SDK Build: Infrastructure failure — swift sdk install crashed (exit code 139) after 5 attempts. Not reproducible locally and not caused by any code change.
  • Linux/Windows localizedName_103036605: Pre-existing DST flakiness in TimeZoneTests (already acknowledged in the codebase with a comment and addressed in Disable a timezone test #1862). Rebased onto main to pick up that fix.

@finagolfin
Copy link
Copy Markdown
Member

@swift-ci test

@ShivaHuang
Copy link
Copy Markdown
Contributor Author

Hi @kperryua, CI is now passing. Friendly ping in case this slipped through. Happy to make any changes if needed!

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.

4 participants