Skip to content

Reactivate CI regression tests for Windows + improve CI #1117

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Jun 18, 2024

Conversation

jarzec
Copy link
Contributor

@jarzec jarzec commented Jun 17, 2024

  • Reactivate CI regression tests on Windows. The test were accidentally disabled (always succeeding without an tests run) though an update of the CI workflow. Update test files:
    • regression-tests/test-results/msvc-2022-c++20/pure2-assert-expected-not-null.cpp.output after the inclusion of .. for non-UFCS,
    • for mixed-default-arguments.cpp2 after the default initialization of function arguments.
  • Add a CI job at the end of the regression tests workflow to aggregate patches generated for each failing run in a single zip fie.
  • Temporarily exclude lines with C:\ from git diff. The reason is that on the GitHub runners the MSVC paths include strange characters that cause git diff to spuriously flag differences. I will raise an issue with with the GitHub team responsible for the Windows runners.

"$patch_file"
"$patch_file" \
--ignore-cr-at-eol \
--ignore-matching-lines="C:\\\\"
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Explanation] This option is excluding lines with Windows paths. On the GitHub runners those paths may include characters that seem to cause trouble with git diff.
This is hopefully a temporary solution.

pure2-assert-expected-not-null.cpp2(14): error C3861: 'unexpected': identifier not found
predefined C++ types (compiler internal)(347): note: see declaration of 'std'
pure2-assert-expected-not-null.cpp2(14): error C2660: 'unexpected': function does not take 1 arguments
C:\Program Files\Microsoft Visual Studio�2nterprise\VC\Tools\MSVC .40.33807\includeh.h(33): note: see declaration of 'unexpected'
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Explanation] This path was always indicated as different despite the test file being updated. My conjecture is that this is due to the strange characters in the path.

@@ -92,7 +92,7 @@ jobs:
bash run-tests.sh -c ${{ matrix.compiler }} -s ${{ matrix.cxx_std }} -d ${{ matrix.stdlib }} -l ${{ matrix.os }}

- name: Run regression tests - Windows version
if: matrix.os == 'windows-latest'
if: startsWith(matrix.os, 'windows')
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Explanation] This line was not updated in a change of this workflow. I've noticed that somewhat accidentally looking at a change that should have caused the test run to fail (e.g. in here) while in fact it showed as successful.

@hsutter
Copy link
Owner

hsutter commented Jun 18, 2024

Thanks!

@hsutter hsutter merged commit 442d17c into hsutter:main Jun 18, 2024
26 checks passed
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.

2 participants