From cf9547e5789d748103cc4d23bc55e9b723fd05c3 Mon Sep 17 00:00:00 2001 From: Eliah Kagan Date: Fri, 8 Nov 2024 15:03:53 -0500 Subject: [PATCH 1/7] Check the specific Windows `GIX_TEST_IGNORE_ARCHIVES=1` failures This modifies the `test-fixtures-windows` job that tests on Windows with `GIX_TEST_IGNORE_ARCHIVES=1` so that, instead of checking that no more than 14 failures occur, it checks that the failing tests are exactly those that are documented in #1358 as expected to fail. The initial check that no tests have *error* status is preserved, with only stylistic changes, and kept separate from the subsequent logic so that the output is clearer. The new steps are no longer conditional on `nextest` having exited with a failure status, since (a) that was probably unnecessary before and definitely unnecessary now, (b) at last for now, the comparison is precise, so it would be strange to pass if the diff were to have changes on *all* lines, and (c) this makes it slightly less likely that #1358 will accidentally stay open even once fixed. The current approach is to actually retrieve the list of tests expected to fail on Windows with `GIX_TEST_IGNORE_ARCHIVES=1` from the #1358 issue body. This has the advantage that it automatically keeps up to date with changes made to that issue description, but this is of course not the only possible approach for populating the expected value. Two changes should be made before this is ready: - As noted in the "FIXME" comment, the job should currently fail becuase the performance test reported to fail in #1358 is not being filtered out from the expected failures list. It's left in as of this commit, to verify that the job is capable of failing. (After that, the performance test should either be filtered out or removed from the list in #1358, but the former approach is currently preferable because I have not done diverse enough testing to check if the failure on my main Windows system is due to that system being too slow rather than a performance bug.) - The scratchwork file should be removed once no longer needed. --- .github/workflows/ci.yml | 42 +++++++++++++++++++++++++++++++-------- scratchwork.ps1 | 43 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 77 insertions(+), 8 deletions(-) create mode 100644 scratchwork.ps1 diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 523e909c8e2..1f210802415 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -95,15 +95,41 @@ jobs: GIX_TEST_IGNORE_ARCHIVES: 1 run: cargo nextest --profile=with-xml run --workspace --no-fail-fast continue-on-error: true - - name: Check how many tests failed - if: steps.nextest.outcome == 'failure' - env: - # See https://github.com/GitoxideLabs/gitoxide/issues/1358. - EXPECTED_FAILURE_COUNT: 14 + - name: Check for errors + run: | + [xml]$junit_xml = Get-Content -Path 'target/nextest/with-xml/junit.xml' + if ($junit_xml.testsuites.errors -ne 0) { exit 1 } + - name: Collect actual failures + run: | + [xml]$junit_xml = Get-Content 'target/nextest/with-xml/junit.xml' + + $actual_failures = $junit_xml.SelectNodes("//testcase[failure]") | + ForEach-Object { "$($_.classname) $($_.name)" } | + Sort-Object + + Write-Output $actual_failures + Set-Content -Path 'actual-failures.txt' -Value $actual_failures + - name: Collect expected failures + run: | + $issue = 1358 # https://github.com/GitoxideLabs/gitoxide/issues/1358 + + $match_info = gh issue --repo GitoxideLabs/gitoxide view $issue --json body --jq .body | + Out-String | + Select-String -Pattern '(?s)```text\r?\n(.*?)```' + + # FIXME: Check that the diff can fail, then filter out performance tests in Where-Object. + $expected_failures = $match_info.Matches.Groups[1].Value -split "`n" | + Where-Object { $_ -match '^\s*FAIL \[' } | + ForEach-Object { $_ -replace '^\s*FAIL \[\s*\d+\.\d+s\]\s*', '' -replace '\s+$', '' } | + Sort-Object + + Write-Output $expected_failures + Set-Content -Path 'expected-failures.txt' -Value $expected_failures + - name: Compare expected and actual failures run: | - [xml]$junit = Get-Content -Path 'target/nextest/with-xml/junit.xml' - if ($junit.testsuites.errors -ne 0) { exit 1 } - if ($junit.testsuites.failures -gt $env:EXPECTED_FAILURE_COUNT) { exit 1 } + # Fail the check if there are any differences, even unexpectedly passing tests, so they can be + # investigated. (If this check is made blocking for PRs, this exact check may need to be changed.) + git --no-pager diff --no-index --exit-code -U1000000 -- expected-failures.txt actual-failures.txt test-32bit: runs-on: ubuntu-latest diff --git a/scratchwork.ps1 b/scratchwork.ps1 new file mode 100644 index 00000000000..89816aaa18e --- /dev/null +++ b/scratchwork.ps1 @@ -0,0 +1,43 @@ +# Omit from all script steps, becuase GHA prepends it. +$ErrorActionPreference = 'stop' + + +Write-Output '====== Output of scratchwork for "Collect actual failures" step: ======' + +[xml]$junit_xml = Get-Content 'target/nextest/with-xml/junit.xml' + +$actual_failures = $junit_xml.SelectNodes("//testcase[failure]") | + ForEach-Object { "$($_.classname) $($_.name)" } | + Sort-Object + +Write-Output $actual_failures +Set-Content -Path 'actual-failures.txt' -Value $actual_failures + + +Write-Output '====== Output of scratchwork for "Collect expected failures" step: ======' + +$issue = 1358 # https://github.com/GitoxideLabs/gitoxide/issues/1358 + +$match_info = gh issue --repo GitoxideLabs/gitoxide view $issue --json body --jq .body | + Out-String | + Select-String -Pattern '(?s)```text\r?\n(.*?)```' + +# FIXME: Check that the diff can fail, then filter out performance tests in Where-Object. +$expected_failures = $match_info.Matches.Groups[1].Value -split "`n" | + Where-Object { $_ -match '^\s*FAIL \[' } | + ForEach-Object { $_ -replace '^\s*FAIL \[\s*\d+\.\d+s\]\s*', '' -replace '\s+$', '' } | + Sort-Object + +Write-Output $expected_failures +Set-Content -Path 'expected-failures.txt' -Value $expected_failures + + +Write-Output '====== Output of scratchwork for "Compare expected and actual failures" step: ======' + +# Fail the check if there are any differences, even unexpectedly passing tests, so they can be +# investigated. (If this check is made blocking for PRs, this exact check may need to be changed.) +git --no-pager diff --no-index --exit-code -U1000000 -- expected-failures.txt actual-failures.txt + + +# Omit from script steps, because GHA appends it. +if ((Test-Path -LiteralPath variable:\LASTEXITCODE)) { exit $LASTEXITCODE } From 29be38f51725e593a5d5c9757389b0161b129775 Mon Sep 17 00:00:00 2001 From: Eliah Kagan Date: Fri, 8 Nov 2024 15:48:40 -0500 Subject: [PATCH 2/7] Let `gh` use `github.token` in "Collect expected failures" MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This is to fix the error: gh: To use GitHub CLI in a GitHub Actions workflow, set the GH_TOKEN environment variable. Example: env: GH_TOKEN: ${{ github.token }} InvalidOperation: D:\a\_temp\ba9d92b7-3a94-4f7c-b541-d19003f40e19.ps1:9 Line | 9 | $expected_failures = $match_info.Matches.Groups[1].Value -split "`n" … | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ | Cannot index into a null array. Error: Process completed with exit code 1. --- .github/workflows/ci.yml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 1f210802415..0bc11f70333 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -110,6 +110,8 @@ jobs: Write-Output $actual_failures Set-Content -Path 'actual-failures.txt' -Value $actual_failures - name: Collect expected failures + env: + GH_TOKEN: ${{ github.token }} run: | $issue = 1358 # https://github.com/GitoxideLabs/gitoxide/issues/1358 From 63473bcde6535714023a18fe1a4b2b54cdcbf759 Mon Sep 17 00:00:00 2001 From: Eliah Kagan Date: Fri, 8 Nov 2024 16:10:14 -0500 Subject: [PATCH 3/7] Colorize the diff Including color in the diff of expected vs. actual failed tests makes the output easier to see (much as color helps in `nextest`). This commit also makes some stylistic changes to the command so it is easier to read. --- .github/workflows/ci.yml | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 0bc11f70333..4865e12d503 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -131,7 +131,8 @@ jobs: run: | # Fail the check if there are any differences, even unexpectedly passing tests, so they can be # investigated. (If this check is made blocking for PRs, this exact check may need to be changed.) - git --no-pager diff --no-index --exit-code -U1000000 -- expected-failures.txt actual-failures.txt + git --no-pager diff --no-index --exit-code --unified=1000000 --color=always -- ` + expected-failures.txt actual-failures.txt test-32bit: runs-on: ubuntu-latest From 0a3f3af0d5aed78f36ea5159552cbd99d7180714 Mon Sep 17 00:00:00 2001 From: Eliah Kagan Date: Fri, 8 Nov 2024 16:43:31 -0500 Subject: [PATCH 4/7] Exclude performance test(s) from expected failures This omits tests containing `performance` (and not as part of a larger "word", not even with `_`) from being expected to fail on CI with `GIX_TEST_IGNORE_ARCHIVES=1` on Windows. Currently there is one such test listed in #1358, `gix-ref-tests::refs packed::iter::performance`. --- .github/workflows/ci.yml | 3 +-- scratchwork.ps1 | 3 +-- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 4865e12d503..08e347c340e 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -119,9 +119,8 @@ jobs: Out-String | Select-String -Pattern '(?s)```text\r?\n(.*?)```' - # FIXME: Check that the diff can fail, then filter out performance tests in Where-Object. $expected_failures = $match_info.Matches.Groups[1].Value -split "`n" | - Where-Object { $_ -match '^\s*FAIL \[' } | + Where-Object { ($_ -match '^\s*FAIL \[') -and ($_ -notmatch '\bperformance\b') } | ForEach-Object { $_ -replace '^\s*FAIL \[\s*\d+\.\d+s\]\s*', '' -replace '\s+$', '' } | Sort-Object diff --git a/scratchwork.ps1 b/scratchwork.ps1 index 89816aaa18e..5fa6c2f102f 100644 --- a/scratchwork.ps1 +++ b/scratchwork.ps1 @@ -22,9 +22,8 @@ $match_info = gh issue --repo GitoxideLabs/gitoxide view $issue --json body --jq Out-String | Select-String -Pattern '(?s)```text\r?\n(.*?)```' -# FIXME: Check that the diff can fail, then filter out performance tests in Where-Object. $expected_failures = $match_info.Matches.Groups[1].Value -split "`n" | - Where-Object { $_ -match '^\s*FAIL \[' } | + Where-Object { ($_ -match '^\s*FAIL \[') -and ($_ -notmatch '\bperformance\b') } | ForEach-Object { $_ -replace '^\s*FAIL \[\s*\d+\.\d+s\]\s*', '' -replace '\s+$', '' } | Sort-Object From b2ce0484118e61cbd04b192e30c9de0f1d15ad6c Mon Sep 17 00:00:00 2001 From: Eliah Kagan Date: Fri, 8 Nov 2024 17:21:50 -0500 Subject: [PATCH 5/7] Remove scratchwork The relevant code is in the `test-fixtures-windows` CI job and it is working (both to fail the job when there is a mismatch and to have the job succeed when there is agreement). --- scratchwork.ps1 | 42 ------------------------------------------ 1 file changed, 42 deletions(-) delete mode 100644 scratchwork.ps1 diff --git a/scratchwork.ps1 b/scratchwork.ps1 deleted file mode 100644 index 5fa6c2f102f..00000000000 --- a/scratchwork.ps1 +++ /dev/null @@ -1,42 +0,0 @@ -# Omit from all script steps, becuase GHA prepends it. -$ErrorActionPreference = 'stop' - - -Write-Output '====== Output of scratchwork for "Collect actual failures" step: ======' - -[xml]$junit_xml = Get-Content 'target/nextest/with-xml/junit.xml' - -$actual_failures = $junit_xml.SelectNodes("//testcase[failure]") | - ForEach-Object { "$($_.classname) $($_.name)" } | - Sort-Object - -Write-Output $actual_failures -Set-Content -Path 'actual-failures.txt' -Value $actual_failures - - -Write-Output '====== Output of scratchwork for "Collect expected failures" step: ======' - -$issue = 1358 # https://github.com/GitoxideLabs/gitoxide/issues/1358 - -$match_info = gh issue --repo GitoxideLabs/gitoxide view $issue --json body --jq .body | - Out-String | - Select-String -Pattern '(?s)```text\r?\n(.*?)```' - -$expected_failures = $match_info.Matches.Groups[1].Value -split "`n" | - Where-Object { ($_ -match '^\s*FAIL \[') -and ($_ -notmatch '\bperformance\b') } | - ForEach-Object { $_ -replace '^\s*FAIL \[\s*\d+\.\d+s\]\s*', '' -replace '\s+$', '' } | - Sort-Object - -Write-Output $expected_failures -Set-Content -Path 'expected-failures.txt' -Value $expected_failures - - -Write-Output '====== Output of scratchwork for "Compare expected and actual failures" step: ======' - -# Fail the check if there are any differences, even unexpectedly passing tests, so they can be -# investigated. (If this check is made blocking for PRs, this exact check may need to be changed.) -git --no-pager diff --no-index --exit-code -U1000000 -- expected-failures.txt actual-failures.txt - - -# Omit from script steps, because GHA appends it. -if ((Test-Path -LiteralPath variable:\LASTEXITCODE)) { exit $LASTEXITCODE } From 067e7d22dc1c6e0bf32b3406211784f7602708bc Mon Sep 17 00:00:00 2001 From: Eliah Kagan Date: Fri, 8 Nov 2024 19:39:26 -0500 Subject: [PATCH 6/7] Clarify comment and code style --- .github/workflows/ci.yml | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 08e347c340e..983f8dca665 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -89,10 +89,10 @@ jobs: - uses: taiki-e/install-action@v2 with: tool: nextest - - name: "Test (nextest)" + - name: Test (nextest) id: nextest env: - GIX_TEST_IGNORE_ARCHIVES: 1 + GIX_TEST_IGNORE_ARCHIVES: '1' run: cargo nextest --profile=with-xml run --workspace --no-fail-fast continue-on-error: true - name: Check for errors @@ -101,7 +101,7 @@ jobs: if ($junit_xml.testsuites.errors -ne 0) { exit 1 } - name: Collect actual failures run: | - [xml]$junit_xml = Get-Content 'target/nextest/with-xml/junit.xml' + [xml]$junit_xml = Get-Content -Path 'target/nextest/with-xml/junit.xml' $actual_failures = $junit_xml.SelectNodes("//testcase[failure]") | ForEach-Object { "$($_.classname) $($_.name)" } | @@ -128,8 +128,8 @@ jobs: Set-Content -Path 'expected-failures.txt' -Value $expected_failures - name: Compare expected and actual failures run: | - # Fail the check if there are any differences, even unexpectedly passing tests, so they can be - # investigated. (If this check is made blocking for PRs, this exact check may need to be changed.) + # Fail on any differences, even unexpectedly passing tests, so they can be investigated. + # (If the job is made blocking for PRs, it may make sense to make this less stringent.) git --no-pager diff --no-index --exit-code --unified=1000000 --color=always -- ` expected-failures.txt actual-failures.txt From 99238a72894595e95d8c03df51fda02285447589 Mon Sep 17 00:00:00 2001 From: Eliah Kagan Date: Sat, 9 Nov 2024 03:14:05 -0500 Subject: [PATCH 7/7] Use a file with `test-fixtures-windows` expected failures Instead of retrieving them from #1358. (See discussion in #1663.) --- .github/workflows/ci.yml | 19 +------------------ ...ndows-expected-failures-see-issue-1358.txt | 14 ++++++++++++++ 2 files changed, 15 insertions(+), 18 deletions(-) create mode 100644 etc/test-fixtures-windows-expected-failures-see-issue-1358.txt diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 983f8dca665..9036a1de167 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -109,29 +109,12 @@ jobs: Write-Output $actual_failures Set-Content -Path 'actual-failures.txt' -Value $actual_failures - - name: Collect expected failures - env: - GH_TOKEN: ${{ github.token }} - run: | - $issue = 1358 # https://github.com/GitoxideLabs/gitoxide/issues/1358 - - $match_info = gh issue --repo GitoxideLabs/gitoxide view $issue --json body --jq .body | - Out-String | - Select-String -Pattern '(?s)```text\r?\n(.*?)```' - - $expected_failures = $match_info.Matches.Groups[1].Value -split "`n" | - Where-Object { ($_ -match '^\s*FAIL \[') -and ($_ -notmatch '\bperformance\b') } | - ForEach-Object { $_ -replace '^\s*FAIL \[\s*\d+\.\d+s\]\s*', '' -replace '\s+$', '' } | - Sort-Object - - Write-Output $expected_failures - Set-Content -Path 'expected-failures.txt' -Value $expected_failures - name: Compare expected and actual failures run: | # Fail on any differences, even unexpectedly passing tests, so they can be investigated. # (If the job is made blocking for PRs, it may make sense to make this less stringent.) git --no-pager diff --no-index --exit-code --unified=1000000 --color=always -- ` - expected-failures.txt actual-failures.txt + etc/test-fixtures-windows-expected-failures-see-issue-1358.txt actual-failures.txt test-32bit: runs-on: ubuntu-latest diff --git a/etc/test-fixtures-windows-expected-failures-see-issue-1358.txt b/etc/test-fixtures-windows-expected-failures-see-issue-1358.txt new file mode 100644 index 00000000000..945b94e97b2 --- /dev/null +++ b/etc/test-fixtures-windows-expected-failures-see-issue-1358.txt @@ -0,0 +1,14 @@ +gix-glob::glob pattern::matching::compare_baseline_with_ours +gix-pathspec::pathspec parse::baseline +gix-pathspec::pathspec parse::valid::glob_negations_are_always_literal +gix-pathspec::pathspec parse::valid::whitespace_in_pathspec +gix-pathspec::pathspec search::files +gix-pathspec::pathspec search::prefixes_are_always_case_sensitive +gix-submodule::submodule file::baseline::common_values_and_names_by_path +gix-submodule::submodule file::is_active_platform::pathspecs_matter_even_if_they_do_not_match +gix-submodule::submodule file::is_active_platform::submodules_with_active_config_are_considered_active_or_inactive +gix-submodule::submodule file::is_active_platform::submodules_with_active_config_override_pathspecs +gix-submodule::submodule file::is_active_platform::without_any_additional_settings_all_are_inactive_if_they_have_a_url +gix-submodule::submodule file::is_active_platform::without_submodule_in_index +gix::gix revision::spec::from_bytes::regex::find_youngest_matching_commit::regex_matches +gix::gix revision::spec::from_bytes::regex::with_known_revision::contained_string_matches_in_unanchored_regex_and_disambiguates_automatically