cut down appveyor's env matrix since we're now running serially

[potatoqualitee]

Original PR #9895 by @niphlod. He said:

This cuts the appveyor matrix in half.
We were splitting scenarios in two parts to leverage appveyor's parallelism, but we're now restricted to one concurrent job, so ? > there's no reason to run splitted parts.

On my test runs, it seems to break for Invoke-DbaDbMirroring on 2016_2017 pretty consistently, let's see how it goes in here.

@claude previously this failed

Path           : Get-DbaBuild/Passing just -Update works, see #6823/works with -Update
Name           : It works with -Update
Result         : Failed
Message        : ErrorRecord: Expected $null or empty, but got [05:00:53][Get-DbaBuildReferenceIndex] Index is stale, last update on: 2025-09-16T00:00:00, try the -Update parameter to fetch the most up to date index.
StackTrace     : 
RawErrorRecord : Expected $null or empty, but got [05:00:53][Get-DbaBuildReferenceIndex] Index is stale, last update on: 2025-09-16T00:00:00, try the -Update parameter to fetch the most up to date index.
Path           : Get-DbaBuild/Validate LastUpdated property/LastUpdated is updated regularly (keeps everybody on their toes)
Name           : It LastUpdated is updated regularly (keeps everybody on their toes)
Result         : Failed
Message        : ErrorRecord: Expected the actual value to be greater than 2025-09-16T05:00:52.6618833+00:00, but got 2025-09-16T00:00:00.0000000.
StackTrace     : 
RawErrorRecord : Expected the actual value to be greater than 2025-09-16T05:00:52.6618833+00:00, but got 2025-09-16T00:00:00.0000000.
Uploading artifact DetailedTestFailures_Pester5.json (2,175 bytes)...100%
2 tests failed.

Any idea? We just consolidated the tests down to fewer runners.

[claude]

PR Review: AppVeyor Matrix Consolidation

Summary

This PR consolidates the AppVeyor test matrix from split 1/2 and 2/2 parts down to single entries per scenario, reducing from 12 matrix configurations to 6. It also disables the Invoke-DbaDbMirroring test on AppVeyor.

---

Code Quality Assessment

✅ Changes Look Good

appveyor.yml:

• Clean removal of duplicate matrix entries
• Proper consolidation - all scenarios now run as single jobs instead of split parts
• Makes sense given AppVeyor now restricts to one concurrent job (no parallelism benefit)

pester.groups.ps1:

• Follows existing code style and patterns
• Correctly adds Invoke-DbaDbMirroring to the appveyor_disabled array
• Proper formatting with aligned entries

---

Test Failure Analysis

❌ Issue: Get-DbaBuild Test Failures

The test failures mentioned in the PR description appear to be unrelated to this PR's changes. The failures are in Get-DbaBuild.Tests.ps1:

1. "It works with -Update" test failure
2. "LastUpdated is updated regularly" test failure

Root Cause: The build reference index file has a future date issue:

• Index file shows: "LastUpdated": "2025-09-16T00:00:00"
• This is in the future relative to when tests ran
• The test expects LastUpdated to be within the last 45 days but not in the future

Location: /bin/dbatools-buildref-index.json:2

Why This Happens:

• The index file likely has a manually set or incorrectly generated LastUpdated timestamp
• Tests at Get-DbaBuild.Tests.ps1:53-56 check that LastUpdated is recent but not future-dated
• When -Update is used with a mocked function, warnings still appear because the index is considered stale

Recommended Fix:
This should be addressed in a separate PR (not this one):

1. Update /bin/dbatools-buildref-index.json with a valid current timestamp
2. Or update the data generation process to use proper dates
3. Consider adding validation to prevent future dates in the index

---

Assessment

Matrix Consolidation ✅

• Correct approach given AppVeyor's current concurrency limitations
• Performance impact: Tests will run longer (serial instead of parallel), but this is unavoidable with AppVeyor's restrictions
• No functional changes to test logic - just reorganization

Invoke-DbaDbMirroring Exclusion ✅

• Good decision to disable if it's failing consistently on 2016_2017
• Follows established pattern for problematic tests
• Properly documented in the appveyor_disabled group

---

Recommendations

1. This PR should be merged as-is - the changes are correct and necessary
2. Create a follow-up issue/PR to fix the Get-DbaBuild test failures by updating the build reference index timestamp
3. Consider documenting why Invoke-DbaDbMirroring is disabled (add a comment or GitHub issue reference)

---

Style Compliance ✅

Checked against CLAUDE.md guidelines:

• ✅ No backticks used
• ✅ Proper formatting maintained
• ✅ Comments preserved
• ✅ Follows existing code patterns
• ✅ YAML indentation correct

---

Verdict: LGTM with the caveat that test failures are pre-existing and unrelated to this PR's changes.

[claude]

Code Review

Summary

This PR consolidates the AppVeyor build matrix by removing the parallelized test splits (1/2 and 2/2 parts). Since AppVeyor now runs serially (one concurrent job), the split execution no longer provides performance benefits and adds unnecessary complexity.

Changes Analysis

✅ appveyor.yml - Build Matrix Consolidation

The removal of duplicate matrix entries is correct and beneficial:

• Reduces 10 matrix entries down to 5
• Removes the now-unnecessary part: 1/2 and part: 2/2 splits
• Each scenario now runs once instead of twice
• Maintains all critical test scenarios (2008R2, 2016, service_restarts, 2016_2017, default)

Impact: This will cut CI runtime in half by eliminating redundant test runs that weren't actually running in parallel.

✅ tests/pester.groups.ps1 - Disabling Flaky Test

Adding 'Invoke-DbaDbMirroring' to appveyor_disabled is appropriate:

• Follows the existing pattern for problematic tests
• Consistent with other disabled tests like Backup-DbaDbCertificate and Test-DbaDbLogShipStatus
• Prevents build failures from known flaky tests

Regarding the Test Failures

The Get-DbaBuild test failures you mentioned appear to be timing-related issues unrelated to these changes:

Expected  or empty, but got [05:00:53][Get-DbaBuildReferenceIndex] Index is stale, 
last update on: 2025-09-16T00:00:00, try the -Update parameter

Root Cause Analysis:
The test at tests/Get-DbaBuild.Tests.ps1:53-56 checks that the build reference index was updated within the last 45 days:

It "LastUpdated is updated regularly (keeps everybody on their toes)" {
    $lastupdate = Get-Date -Date $IdxRef.LastUpdated
    $lastupdate | Should -BeGreaterThan (Get-Date).AddDays(-45)
}

Why it's failing:

• The build reference index file (bin/dbatools-buildref-index.json) has a LastUpdated date of 2025-09-16
• Today's date is 2025-10-31 (from environment context)
• 2025-09-16 is 45+ days old, failing the test
• This is a data staleness issue, not related to the matrix consolidation

This failure would occur regardless of the PR changes - it's due to the build reference index not being updated recently enough.

Recommendations

1. ✅ Approve the matrix consolidation - The code changes are correct and improve CI efficiency

2. ⚠️ Address the Get-DbaBuild test failure separately - This requires either:

   • Updating the build reference index (running Update-DbaBuildReference with proper data)
   • Adjusting the 45-day threshold if it's too strict
   • This should be handled in a separate PR/issue

3. ✅ The Invoke-DbaDbMirroring disabling is appropriate - Per the original author's note, it consistently breaks on 2016_2017 scenarios

Code Quality Assessment

• ✅ No code style issues
• ✅ No security concerns
• ✅ Changes follow dbatools conventions
• ✅ Proper comment preservation (AppVeyor comments intact)
• ✅ No performance regressions expected
• ✅ Configuration changes are minimal and focused

Verdict

APPROVE with the understanding that the Get-DbaBuild test failure is a separate data staleness issue that should be tracked independently. The PR changes themselves are sound and will improve CI efficiency.

The test failure is not caused by this PR - it's a pre-existing issue with stale build reference data that needs attention but shouldn't block this optimization.

[niphlod]

ahem, are you trying to ask claude a difficult question ? SQL isn't releasing patches in a while so we need to bump the date at the head just to confirm we're keeping it up to date

[niphlod]

ha. that part it got right. other things .... not really but anyway... why dbmirroring was running fine before and now it doesn't ? we shouldn't lightheartedly disable tests that were working before

[potatoqualitee]

@niphlod i agree but it’s torturous waiting 4 hours per PR. So I’d like to get your PR in then fix it. The command didn’t break. We can hold off on edits to that until it’s fixed and/or move it to github actions.

[potatoqualitee]

regarding the test, i didnt wanna figure out why it failed. usually when a test is out of date, we have like 87 failures and not just one 😁