Skip to content

Latest commit

 

History

History
543 lines (446 loc) · 14.9 KB

File metadata and controls

543 lines (446 loc) · 14.9 KB

TinyBDD Dogfooding Review for JD.MSBuild.Fluent

Date: 2026-01-20
Reviewer: Automated Analysis
TinyBDD Current Version: 0.18.1 → Latest: Check TinyBDD releases
PatternKit Status: Repository does not exist yet


Executive Summary

JD.MSBuild.Fluent is partially dogfooding TinyBDD but missing several key features and best practices documented in TinyBDD v0.18.1+. PatternKit does not exist, so no review possible for that library.

Quick Stats

  • ✅ TinyBDD.Xunit v0.18.1 referenced
  • ✅ 184+ TinyBDD method calls across tests
  • ⚠️ 0/15 test classes use [Feature] attribute
  • ⚠️ 0/15 test classes use [Scenario] attribute
  • ⚠️ 0/15 test methods use .Finally() for cleanup
  • ⚠️ 1/15 test files doesn't inherit from TinyBddXunitBase (ValidationTests.cs)
  • ⚠️ No use of feature setup/teardown lifecycle hooks
  • ⚠️ No use of tags for test organization

📋 Detailed Findings

1. Missing [Feature] and [Scenario] Attributes

Current State:

/// <summary>Feature: Fluent API - Package Definition</summary>
public sealed class BddFluentApiTests(ITestOutputHelper output) : TinyBddXunitBase(output)
{
    [Fact]
    public async Task Scenario_Define_basic_package()

Best Practice (from TinyBDD docs):

[Feature("Fluent API - Package Definition")]
public sealed class BddFluentApiTests(ITestOutputHelper output) : TinyBddXunitBase(output)
{
    [Scenario("Define basic package")]
    [Fact]
    public async Task DefineBasicPackage()

Benefits:

  • Attributes are automatically logged to test output
  • xUnit traits/tags integration for filtering
  • Better reporting and discoverability
  • Follows TinyBDD conventions

Action Items:

  • Add [Feature] to all 14 BDD test classes
  • Add [Scenario] to all test methods
  • Remove redundant naming (e.g., "Scenario_" prefix, "Tests" suffix in class names)

2. No Use of .Finally() for Resource Cleanup

Current State:

[Fact]
public async Task Scenario_Generate_complete_package()
{
    string? tempDir = null;
    try
    {
        await Given("a complete package definition", () =>
            {
                var pkg = Package.Define("CompletePackage")
                    .Build();
                tempDir = Path.Combine(Path.GetTempPath(), $"BddTest_{Guid.NewGuid():N}");
                Directory.CreateDirectory(tempDir);
                return (pkg, tempDir);
            })
            .When("generating package to disk", ctx =>
            {
                var emitter = new MsBuildPackageEmitter();
                emitter.Emit(ctx.pkg, ctx.tempDir);
                return ctx.tempDir;
            })
            // ... assertions ...
            .AssertPassed();
    }
    finally
    {
        if (tempDir != null && Directory.Exists(tempDir))
            Directory.Delete(tempDir, recursive: true);
    }
}

Best Practice:

[Scenario("Generate complete package")]
[Fact]
public async Task GenerateCompletePackage()
{
    await Given("a complete package definition", () =>
        {
            var pkg = Package.Define("CompletePackage").Build();
            var tempDir = Path.Combine(Path.GetTempPath(), $"BddTest_{Guid.NewGuid():N}");
            Directory.CreateDirectory(tempDir);
            return (pkg, tempDir);
        })
        .Finally("cleanup temp directory", ctx =>
        {
            if (Directory.Exists(ctx.tempDir))
                Directory.Delete(ctx.tempDir, recursive: true);
        })
        .When("generating package to disk", ctx =>
        {
            var emitter = new MsBuildPackageEmitter();
            emitter.Emit(ctx.pkg, ctx.tempDir);
            return ctx.tempDir;
        })
        // ... assertions ...
        .AssertPassed();
}

Benefits:

  • Cleanup guaranteed even on assertion failures
  • Cleaner test code (no try/finally blocks)
  • Cleanup shows in BDD output
  • Multiple Finally handlers can be registered

Action Items:

  • Replace all try/finally blocks with .Finally()
  • Review ~5+ tests that create temp files/directories

3. Non-BDD Test Class (ValidationTests.cs)

Current State:

public sealed class ValidationTests
{
  [Fact]
  public void Renderer_throws_on_invalid_target()
  {
    var project = new MsBuildProject();
    project.Elements.Add(new MsBuildTarget { Name = string.Empty });

    var renderer = new MsBuildXmlRenderer();
    Action act = () => renderer.RenderToString(project);

    var ex = act.Should().Throw<MsBuildValidationException>().Which;
    ex.Errors.Should().Contain(e => e.Contains("Target name is required."));
  }

Best Practice:

[Feature("MSBuild Validation")]
public sealed class ValidationTests(ITestOutputHelper output) : TinyBddXunitBase(output)
{
  [Scenario("Renderer throws on invalid target")]
  [Fact]
  public async Task RendererThrowsOnInvalidTarget()
  {
    await Given("a project with empty target name", () => 
        {
            var project = new MsBuildProject();
            project.Elements.Add(new MsBuildTarget { Name = string.Empty });
            return project;
        })
        .When("rendering to XML", project =>
        {
            var renderer = new MsBuildXmlRenderer();
            try
            {
                renderer.RenderToString(project);
                return (success: true, error: (MsBuildValidationException?)null);
            }
            catch (MsBuildValidationException ex)
            {
                return (success: false, error: ex);
            }
        })
        .Then("should throw validation exception", result => !result.success)
        .And("error contains target name message", result =>
            result.error?.Errors.Any(e => e.Contains("Target name is required.")) == true)
        .AssertPassed();
  }

Benefits:

  • Consistent BDD style across all tests
  • Better readability and maintainability
  • Step-by-step execution visibility in test output

Action Items:

  • Convert ValidationTests.cs to TinyBDD style
  • Add constructor with ITestOutputHelper
  • Inherit from TinyBddXunitBase

4. No Feature Lifecycle Hooks

TinyBDD supports feature-level setup/teardown that runs once per test class:

Example from TinyBDD docs:

[Feature("Calculator with shared state")]
public class CalculatorFeatureTests : TinyBddXunitBase
{
    public CalculatorFeatureTests(ITestOutputHelper output) : base(output) { }

    protected override ScenarioChain<object>? ConfigureFeatureSetup()
    {
        return Given("a shared calculator instance", () => (object)new Calculator());
    }

    protected override ScenarioChain<object>? ConfigureFeatureTeardown()
    {
        return Given("cleanup resources", () =>
        {
            // Cleanup code
            return new object();
        });
    }

    [Scenario("First test uses feature state")]
    [Fact]
    public async Task FirstTest()
    {
        await GivenFeature<Calculator>("the calculator")
            .When("adding numbers", calc => calc.Add(1, 2))
            .Then("result is correct", result => result == 3)
            .AssertPassed();
    }
}

Benefits:

  • Share expensive setup across tests (e.g., test containers, temp directories)
  • Reduced test execution time
  • Better resource management

Action Items:

  • Identify test classes that repeat setup (e.g., temp directory creation)
  • Implement ConfigureFeatureSetup() for shared state
  • Use GivenFeature<T>() to access shared state

5. No Use of Tags

Best Practice:

[Feature("MSBuild Generation")]
public class GenerationTests : TinyBddXunitBase
{
    [Scenario("Generate props file")]
    [Fact]
    [Tag("generation")]
    [Tag("props")]
    [Tag("smoke")]
    public async Task GeneratePropsFile()
    {
        await Given("a package definition", () => Package.Define("Test"))
            .When("generating props", /* ... */)
            .Then("props file exists", /* ... */)
            .AssertPassed();
    }
}

Or programmatically:

await Given("a package", () => Package.Define("Test"))
    .When("generating", pkg => { ctx.AddTag("generation"); return pkg; })
    .Then("succeeds", /* ... */)
    .AssertPassed();

Benefits:

  • Filter tests by category (e.g., dotnet test --filter Tag=smoke)
  • Organize tests for CI/CD pipelines
  • Better test reporting and analytics

Action Items:

  • Add tags to categorize tests (e.g., "smoke", "integration", "generation", "validation")
  • Document tag conventions in CONTRIBUTING.md

6. Performance Optimization Not Opted Out Where Needed

TinyBDD v1.1+ includes automatic compile-time optimization via source generator. Most tests should benefit, but some may need [DisableOptimization]:

When to opt-out:

  • Tests using observers/hooks (none found)
  • Tests needing full pipeline features
  • When debugging step execution

Current State:

  • No tests use [DisableOptimization]
  • Tests appear compatible with optimization
  • ✅ This is likely fine

Action Items:

  • Document that tests are optimized by default
  • Add [DisableOptimization] if debugging issues arise

📊 Compliance Scorecard

Category Current Target Status
Feature Attributes 0/15 15/15 🔴
Scenario Attributes 0/~100 100/100 🔴
Finally() Usage 0 ~5 🔴
TinyBddXunitBase Inheritance 14/15 15/15 🟡
Feature Lifecycle 0/15 ~3/15 🔴
Tags 0 ~50 🔴
Version 0.18.1 Latest 🟡

Overall: 🔴 Needs Improvement (2/7 categories passing)


🎯 Priority Action Plan

High Priority (Do First)

  1. Add [Feature] and [Scenario] attributes to all test classes/methods

    • Estimated effort: 30 minutes
    • High impact on test organization and reporting
  2. Replace try/finally with .Finally() in all tests with cleanup

    • Estimated effort: 15 minutes
    • Improves code clarity and reliability
  3. Convert ValidationTests.cs to TinyBDD style

    • Estimated effort: 20 minutes
    • Ensures consistency across test suite

Medium Priority

  1. Add feature lifecycle hooks to tests with repeated setup

    • Estimated effort: 1 hour
    • Improves test performance and maintainability
  2. Add tags for test categorization

    • Estimated effort: 30 minutes
    • Enables better test filtering and CI/CD integration

Low Priority

  1. Update to latest TinyBDD version

    • Check for new features and bug fixes
    • Update documentation if breaking changes
  2. Create PatternKit (when available)

    • Currently doesn't exist
    • Defer until library is created

📚 Code Patterns to Improve

Pattern 1: Inconsistent Step Descriptions

Current:

await Given("start", () => 5)
    .When("double", x => x * 2)
    .Then(">= 10", v => v >= 10)  // Symbol-based description

Better:

await Given("a number 5", () => 5)
    .When("doubled", x => x * 2)
    .Then("result is at least 10", v => v >= 10)  // Natural language

Pattern 2: Complex Assertions in Then/And

Current:

.Then("package ID should be set", def => def.Id == "MyPackage")

Better (with FluentAssertions):

.Then("package ID should be MyPackage", def =>
{
    def.Id.Should().Be("MyPackage");
    return true;
})

Or leverage FluentAssertions more:

.Then("package is valid", def =>
{
    def.Id.Should().Be("MyPackage");
    def.Description.Should().NotBeNullOrEmpty();
    def.Props.Should().NotBeNull();
    return true;
})

Pattern 3: Long Anonymous Functions

Current:

await Given("a package", () => Package.Define("Test")
        .Props(p => p.Property<TProp>("Val"))
        .Targets(t => t.Target<TTarget>(tgt => tgt.Message("Hi")))
        .Build())

Better:

static PackageDefinition CreateTestPackage() =>
    Package.Define("Test")
        .Props(p => p.Property<TProp>("Val"))
        .Targets(t => t.Target<TTarget>(tgt => tgt.Message("Hi")))
        .Build();

await Given("a package", CreateTestPackage)

Benefits:

  • Easier to read
  • Better for debugging
  • Reduces allocations (TinyBDD docs recommendation)

🔍 Example Refactoring

Before

/// <summary>Feature: Fluent API - Package Definition</summary>
public sealed class BddFluentApiTests(ITestOutputHelper output) : TinyBddXunitBase(output)
{
    [Fact]
    public async Task Scenario_Define_basic_package()
    {
        await Given("a package ID", () => "MyPackage")
            .When("defining with Package.Define", id => Package.Define(id).Build())
            .Then("package ID should be set", def => def.Id == "MyPackage")
            .AssertPassed();
    }
}

After

[Feature("Fluent API - Package Definition")]
public sealed class BddFluentApiTests(ITestOutputHelper output) : TinyBddXunitBase(output)
{
    [Scenario("Define basic package")]
    [Fact]
    [Tag("fluent-api")]
    [Tag("smoke")]
    public async Task DefineBasicPackage()
    {
        await Given("a package ID", () => "MyPackage")
            .When("defining with Package.Define", DefinePackage)
            .Then("package ID matches expected", VerifyPackageId)
            .AssertPassed();
    }

    private static PackageDefinition DefinePackage(string id) => 
        Package.Define(id).Build();

    private static bool VerifyPackageId(PackageDefinition def)
    {
        def.Id.Should().Be("MyPackage");
        return true;
    }
}

📦 PatternKit Review

Status: ❌ Repository does not exist at C:\git\PatternKit

Recommendations when created:

  • Define patterns for common MSBuild scenarios
  • Create builder utilities for flat, declarative code
  • Provide LINQ-style query extensions for IR traversal
  • Add validation pattern helpers
  • Document all patterns with usage examples

Suggested Patterns:

  • Result<T, TError> for better error handling
  • Option<T> for null safety
  • Builder pattern utilities
  • Functional composition helpers
  • Validation combinators

✅ Next Steps

  1. Create GitHub Issues

    • One issue per high-priority item
    • Link to this review document
  2. Update TinyBDD

    • Check for latest version
    • Review changelog for new features
  3. Refactor Tests (Suggested Order)

    • Start with BddFluentApiTests.cs (smallest)
    • Apply patterns to other test classes
    • Update ValidationTests.cs last
  4. Documentation

    • Add TinyBDD usage guide to project
    • Document test organization and tagging strategy
    • Link to TinyBDD docs in CONTRIBUTING.md
  5. CI/CD Integration

    • Add test filtering by tags
    • Run smoke tests on PR
    • Full test suite on main branch

📖 References


Review Complete. Priority: Implement High Priority items in next sprint.