feat: implement locale-specific build workflow

[thomhurst]

No description provided.

[claude]

Claude finished @thomhurst's task —— View job

---

PR Review Complete

This PR implements locale-specific testing to ensure TUnit handles decimal values correctly across different cultures. Here's my comprehensive review:

Todo List:

• Read repository guidelines (CLAUDE.md)
• Review new workflow file (.github/workflows/dotnet-build-different-locale.yml)
• Review test file changes (TUnit.TestProject/DecimalArgumentTests.cs)
• Analyze code quality and best practices
• Check for potential bugs or issues
• Evaluate performance considerations
• Assess security concerns
• Review test coverage
• Provide comprehensive feedback

---

📋 Review Findings

✅ Strengths

1. Important Testing Addition: This PR addresses a critical testing gap by validating decimal handling across different locales (French, Polish, German), which is essential for international applications.

2. Clean Workflow Design: The GitHub Actions workflow properly isolates locale-specific testing and follows GitHub Actions best practices.

3. Comprehensive Test Coverage: The test file covers edge cases including precision preservation, min/max values, and multiple decimal scenarios.

⚠️ Issues and Recommendations

High Priority

1. Missing Test Execution in Workflow (.github/workflows/dotnet-build-different-locale.yml:51-52)

   • The workflow only builds but doesn't run tests. For locale testing to be effective, tests must actually execute.
   • Fix: Add a test step:

   - name: Test
     run: dotnet test -c Release --no-build
     working-directory: TUnit.TestProject

2. Incomplete Locale Coverage

   • Consider adding locales with different decimal separators (e.g., ar-SA for Arabic, hi-IN for Hindi) to ensure comprehensive coverage.

Medium Priority

3. Potential Locale Setup Issues (.github/workflows/dotnet-build-different-locale.yml:35-49)

   • The locale generation might fail silently if locales aren't available in the Ubuntu image.
   • Recommendation: Add verification:

   - name: Verify locale setup
     run: locale -a | grep -i "${LOCALE}"

4. Culture-Sensitive Test Enhancement (TUnit.TestProject/DecimalArgumentTests.cs:24-25)

   • The precision tests use CultureInfo.InvariantCulture which defeats the purpose of locale testing.
   • Recommendation: Test both invariant and current culture formatting:

   await Assert.That(debit.ToString(CultureInfo.CurrentCulture)).IsNotEmpty();
   await Assert.That(debit.ToString(CultureInfo.InvariantCulture)).IsEqualTo("2000.00000000000000001");

Low Priority

5. Workflow Naming (.github/workflows/dotnet-build-different-locale.yml:1)

   • The workflow name ".NET" is generic and doesn't reflect its locale-specific purpose.
   • Suggestion: Rename to "Locale Testing" or "Cross-Culture Build"

6. Performance Consideration

   • The workflow runs for every PR on main branch. Consider adding path filters to only run when relevant code changes.

🔒 Security Assessment

• ✅ No security concerns identified
• Workflow uses trusted actions with proper versioning
• No sensitive data or credentials exposed

🚀 Performance Assessment

• ✅ Minimal performance impact
• Adds ~3 minutes per locale to CI pipeline
• Parallel execution keeps total time reasonable

🧪 Test Coverage Assessment

• ✅ Excellent decimal test coverage in the test file
• ❌ Missing actual test execution in the workflow
• ⚠️ Limited culture-specific assertions

🎯 Branch Alignment Check

• Branch name bug/3194 suggests this addresses a specific issue, but no description links it to the bug
• Recommendation: Add PR description explaining what bug this fixes

📝 Summary

This is a valuable addition for ensuring cross-cultural compatibility, but the workflow needs to actually run tests to be effective. The test coverage is comprehensive, though some assertions could be more culture-aware. Overall approach is sound and aligns with TUnit's quality standards.

Priority: Fix the missing test execution step before merging.

---

To fix the problem and follow least-privilege guidelines, explicitly set the minimal permissions for the workflow or the modularpipeline job. Since there is only one job, and it does not need write permissions, setting

permissions:
  contents: read

either at the workflow root (applies to all jobs), or within the job (applies just to that job) will resolve the issue and satisfy CodeQL. For clarity and maintainability, put this at the root, after name: .NET, so it applies globally and is easily discoverable by future maintainers. No other changes required.

[claude]

Claude finished @thomhurst's task —— View job

---

📋 PR Review: Locale-specific Build Workflow

Todo List:

• Read repository guidelines (CLAUDE.md)
• Review new workflow file (.github/workflows/dotnet-build-different-locale.yml)
• Review analyzer changes (TUnit.Analyzers/TestDataAnalyzer.cs)
• Review source generator changes (TUnit.Core.SourceGenerator/CodeGenerators/Formatting/TypedConstantFormatter.cs)
• Review new decimal parsing helper (TUnit.Core/Helpers/DecimalParsingHelper.cs)
• Review test file changes (TUnit.TestProject/DecimalArgumentTests.cs)
• Analyze code quality and best practices
• Check for potential bugs or issues
• Evaluate performance considerations
• Assess security concerns
• Review test coverage
• Address GitHub Advanced Security findings
• Provide comprehensive feedback

---

🎯 Overall Assessment

This PR implements locale-specific testing to ensure TUnit handles decimal parsing correctly across different cultures. The implementation demonstrates strong engineering practices with comprehensive fallback mechanisms and thorough test coverage.

✅ Strengths

1. Critical Feature Addition: Addresses decimal parsing across different locales (French, Polish, German), which is essential for international applications where decimal separators vary (, vs .).

2. Robust Fallback Strategy: The DecimalParsingHelper implements a smart fallback pattern:

   • First attempts parsing with current culture
   • Falls back to invariant culture if needed
   • Provides detailed error messages on failure

3. Source Generator Integration: Proper integration in TypedConstantFormatter.cs:204 ensures generated code uses culture-aware parsing for string-to-decimal conversions.

4. Comprehensive Test Coverage: DecimalArgumentTests.cs covers edge cases including precision preservation, min/max values, and multiple decimal scenarios.

5. Analyzer Enhancement: Updates to TestDataAnalyzer.cs:942-968 allow compile-time validation of string-to-decimal conversions in both dot and comma formats.

⚠️ Issues Requiring Attention

Critical Issues

1. Workflow Missing Test Execution (.github/workflows/dotnet-build-different-locale.yml:50-52)

   • Problem: Workflow only builds but doesn't run tests, making locale testing ineffective
   • Fix: Add test step after build:

   - name: Test
     run: dotnet test -c Release --no-build
     working-directory: TUnit.TestProject

2. Missing Permissions Block (Security Alert)

   • Problem: GitHub Advanced Security flagged missing permissions
   • Fix: Add permissions block:

   permissions:
     contents: read

Medium Priority Issues

3. Potential Silent Locale Failures (.github/workflows/dotnet-build-different-locale.yml:35-49)

   • Problem: Locale generation might fail silently if locales aren't available
   • Recommendation: Add verification step:

   - name: Verify locale setup
     run: locale -a | grep -E "(fr_FR|pl_PL|de_DE)"

4. Limited Culture Testing in Tests (DecimalArgumentTests.cs:24-25)

   • Problem: Tests use CultureInfo.InvariantCulture which defeats locale testing purpose
   • Improvement: Add culture-specific assertions:

   await Assert.That(debit.ToString(CultureInfo.CurrentCulture)).IsNotEmpty();
   await Assert.That(debit.ToString(CultureInfo.InvariantCulture)).IsEqualTo("2000.00000000000000001");

Minor Issues

5. Generic Workflow Name (.github/workflows/dotnet-build-different-locale.yml:1)

   • Suggestion: Rename from ".NET" to "Cross-Culture Testing" for clarity

6. Branch Name Mismatch: Branch bug/3194 suggests bug fix but no issue description provided

🔒 Security Assessment

• ✅ No security vulnerabilities identified
• Uses trusted GitHub actions with proper versioning
• No credential exposure or injection risks
• String parsing uses safe .NET methods

🚀 Performance Assessment

• ✅ Minimal performance impact
• Culture-aware parsing adds negligible overhead
• Workflow adds ~3 minutes per locale to CI (acceptable for quality assurance)
• Compile-time validation prevents runtime errors

🧪 Test Coverage Assessment

• ✅ Excellent decimal parsing coverage
• ✅ Edge cases well covered (precision, min/max values)
• ❌ Missing actual test execution in workflow
• ⚠️ Limited culture-specific test assertions

📝 Code Quality Assessment

Excellent Practices

• Proper use of CultureInfo.InvariantCulture for formatting in source generator
• Comprehensive error handling with descriptive messages
• Modern C# patterns following repository guidelines
• Good separation of concerns between helper, analyzer, and generator

Follows TUnit Guidelines

• ✅ Maintains behavioral parity between source-generated and reflection modes
• ✅ Uses modern C# patterns and formatting
• ✅ Performance-conscious implementation
• ✅ Meaningful names over comments

🎯 Recommendations

1. High Priority: Add test execution to workflow and permissions block
2. Medium Priority: Enhance test assertions to be more culture-aware
3. Low Priority: Add locale verification and improve workflow naming
4. Documentation: Add PR description linking to issue Arguments with a bool working in 0.60.1 fail in 0.60.15+ #3194

💭 Summary

This is a valuable addition that addresses an important internationalization concern. The core implementation is solid with good error handling and fallback mechanisms. However, the workflow needs to actually run tests to be effective. The changes align well with TUnit's quality standards and performance requirements.

Priority: Fix the missing test execution and permissions before merging.

---

[claude]

Claude finished @thomhurst's task —— View job

---

Claude finished reviewing the PR

📋 Review Complete

• Read repository guidelines (CLAUDE.md)
• Review new workflow file (.github/workflows/dotnet-build-different-locale.yml)
• Review analyzer changes (TUnit.Analyzers/TestDataAnalyzer.cs)
• Review source generator changes (TUnit.Core.SourceGenerator/CodeGenerators/Formatting/TypedConstantFormatter.cs)
• Review new decimal parsing helper (TUnit.Core/Helpers/DecimalParsingHelper.cs)
• Review test file changes (TUnit.TestProject/DecimalArgumentTests.cs)
• Review public API changes
• Analyze code quality and best practices
• Check for potential bugs or issues
• Evaluate performance considerations
• Assess security concerns
• Review test coverage
• Address GitHub Advanced Security findings
• Provide comprehensive feedback

---

🎯 Overall Assessment

This PR implements locale-specific testing to ensure TUnit handles decimal parsing correctly across different cultures. The implementation demonstrates strong engineering practices with comprehensive fallback mechanisms and thorough test coverage.

✅ Strengths

1. Critical Feature Addition: Addresses decimal parsing across different locales (French, Polish, German), which is essential for international applications where decimal separators vary (, vs .).

2. Robust Fallback Strategy: The DecimalParsingHelper implements a smart fallback pattern:

   • First attempts parsing with current culture
   • Falls back to invariant culture if needed
   • Provides detailed error messages on failure

3. Source Generator Integration: Proper integration in TypedConstantFormatter.cs:204 ensures generated code uses culture-aware parsing for string-to-decimal conversions.

4. Comprehensive Test Coverage: DecimalArgumentTests.cs covers edge cases including precision preservation, min/max values, and multiple decimal scenarios.

5. Analyzer Enhancement: Updates to TestDataAnalyzer.cs:942-968 allow compile-time validation of string-to-decimal conversions in both dot and comma formats.

⚠️ Issues Requiring Attention

Critical Issues

1. Workflow Missing Test Execution (.github/workflows/dotnet-build-different-locale.yml:50-52)

   • Problem: Workflow only builds but doesn't run tests, making locale testing ineffective
   • Fix: Add test step after build:

   - name: Test
     run: dotnet test -c Release --no-build
     working-directory: TUnit.TestProject

2. Missing Permissions Block (Security Alert)

   • Problem: GitHub Advanced Security flagged missing permissions
   • Fix: Add permissions block:

   permissions:
     contents: read

Medium Priority Issues

3. Potential Silent Locale Failures (.github/workflows/dotnet-build-different-locale.yml:35-49)

   • Problem: Locale generation might fail silently if locales aren't available
   • Recommendation: Add verification step:

   - name: Verify locale setup
     run: locale -a | grep -E "(fr_FR|pl_PL|de_DE)"

4. Limited Culture Testing in Tests (DecimalArgumentTests.cs:24-25)

   • Problem: Tests use CultureInfo.InvariantCulture which defeats locale testing purpose
   • Improvement: Add culture-specific assertions:

   await Assert.That(debit.ToString(CultureInfo.CurrentCulture)).IsNotEmpty();
   await Assert.That(debit.ToString(CultureInfo.InvariantCulture)).IsEqualTo("2000.00000000000000001");

Minor Issues

5. Generic Workflow Name (.github/workflows/dotnet-build-different-locale.yml:1)

   • Suggestion: Rename from ".NET" to "Cross-Culture Testing" for clarity

6. Branch Name Mismatch: Branch bug/3194 suggests bug fix but no issue description provided

🔒 Security Assessment

• ✅ No security vulnerabilities identified
• Uses trusted GitHub actions with proper versioning
• No credential exposure or injection risks
• String parsing uses safe .NET methods

🚀 Performance Assessment

• ✅ Minimal performance impact
• Culture-aware parsing adds negligible overhead
• Workflow adds ~3 minutes per locale to CI (acceptable for quality assurance)
• Compile-time validation prevents runtime errors

🧪 Test Coverage Assessment

• ✅ Excellent decimal parsing coverage
• ✅ Edge cases well covered (precision, min/max values)
• ❌ Missing actual test execution in workflow
• ⚠️ Limited culture-specific test assertions

📝 Code Quality Assessment

Excellent Practices

• Proper use of CultureInfo.InvariantCulture for formatting in source generator
• Comprehensive error handling with descriptive messages
• Modern C# patterns following repository guidelines
• Good separation of concerns between helper, analyzer, and generator

Follows TUnit Guidelines

• ✅ Maintains behavioral parity between source-generated and reflection modes
• ✅ Uses modern C# patterns and formatting
• ✅ Performance-conscious implementation
• ✅ Meaningful names over comments

🎯 Recommendations

1. High Priority: Add test execution to workflow and permissions block
2. Medium Priority: Enhance test assertions to be more culture-aware
3. Low Priority: Add locale verification and improve workflow naming
4. Documentation: Add PR description linking to issue Arguments with a bool working in 0.60.1 fail in 0.60.15+ #3194

💭 Summary

This is a valuable addition that addresses an important internationalization concern. The core implementation is solid with good error handling and fallback mechanisms. However, the workflow needs to actually run tests to be effective. The changes align well with TUnit's quality standards and performance requirements.

Priority: Fix the missing test execution and permissions before merging.

---