Skip to content
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

CI: skip cli-suite on Windows due to #9571 #10257

Merged
merged 1 commit into from
Aug 13, 2024
Merged

Conversation

ulysses4ever
Copy link
Collaborator

Further investigation of #9571 is needed, but we can take a breathe in the meantime.

Template B: This PR does not modify behaviour or interface

E.g. the PR only touches documentation or tests, does refactorings, etc.

Include the following checklist in your PR:

  • Patches conform to the coding conventions.
  • Is this a PR that fixes CI? If so, it will need to be backported to older cabal release branches (ask maintainers for directions).

@ulysses4ever ulysses4ever added squash+merge me Tell Mergify Bot to squash-merge merge delay passed Applied (usually by Mergify) when PR approved and received no updates for 2 days labels Aug 13, 2024
@ulysses4ever
Copy link
Collaborator Author

If no one objects, I propose to merge it skipping the 2-day delay. Once we get the two approvals of course.

@@ -210,6 +210,7 @@ jobs:
run: sh validate.sh $FLAGS -s cli-tests

- name: Validate cli-suite
if: runner.os != 'Windows'
Copy link
Collaborator

Choose a reason for hiding this comment

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

See line 177 for how to do this properly; I think that won't work as is (runner.os won't be expanded).

Copy link
Collaborator

Choose a reason for hiding this comment

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

Okay, actually it looks like it did work (skipped cli-suite only on Windows) but we probably want to be consistent.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Line 177 is a part of a bash expression, and I act on the yaml level. So, I'm not sure why they should be consistent necessarily: they're pretty different notations...

@mergify mergify bot merged commit 30d2a38 into master Aug 13, 2024
50 checks passed
@mergify mergify bot deleted the ulysses4ever-patch-1 branch August 13, 2024 17:35
@geekosaur
Copy link
Collaborator

Do we want to backport this / will this affect CI on 3.12 branch?

@geekosaur
Copy link
Collaborator

@mergify backport 3.12

Copy link
Contributor

mergify bot commented Sep 13, 2024

backport 3.12

✅ Backports have been created

mergify bot pushed a commit that referenced this pull request Sep 13, 2024
geekosaur pushed a commit that referenced this pull request Sep 13, 2024
mergify bot added a commit that referenced this pull request Sep 13, 2024
…10305)

* hackage-tests: Add --index-state argument to fix the cabal files

We need to fix the index-state we test against so a new bad cabal file
doesn't take down the CI for everyone.

Towards #10284

(cherry picked from commit 8e4d167)

* ci: Fix --index-state for hackage roundtrip tests

As a principle, tests which are required for CI to pass should be
reproducible and not depending on external resources changes or being
modified. The hackage tests currently violate this by depending on the
latest index state from hackage. This is problematic because until the
test is fixed all merges into master are blocked. Even though the
patches in question have nothing to do with the test.

It would be more suitable for a nightly job to run on the latest index
and for normal CI to run with a fixed index which is updated
periodically in a controlled manner.

Fixes #10284

(cherry picked from commit 31507b1)

* Re-enable Windows CI

(cherry picked from commit 4aade2d)

* CI: skip cli-suite on Windows due to #9571 (#10257)

(cherry picked from commit 30d2a38)

---------

Co-authored-by: Matthew Pickering <[email protected]>
Co-authored-by: Javier Sagredo <[email protected]>
Co-authored-by: Artem Pelenitsyn <[email protected]>
erikd pushed a commit to erikd/cabal that referenced this pull request Jan 9, 2025
erikd pushed a commit to erikd/cabal that referenced this pull request Jan 9, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
merge delay passed Applied (usually by Mergify) when PR approved and received no updates for 2 days priority: high 🔥 squash+merge me Tell Mergify Bot to squash-merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants