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: makes relevant CI tests multi-platform #230

Merged
merged 1 commit into from
Nov 1, 2024
Merged

Conversation

claymcleod
Copy link
Member

@claymcleod claymcleod commented Oct 21, 2024

We should make sure wdl is always tested to work on all major platforms. To that end, I've updated the CI here to ensure that it always is. Some steps don't need to be run multi-platform, others clearly do, and then there are a few that are in-between (e.g., linting).

In both the "definitely needs to be run on all platforms" and the "in-between" ones, I ended up just enabling multi-platform tests to be safe. For example, for linting, some lints may only show up when certain platform-based configuration options are available. This is only going to become more relevant as we add more platform-specific things (like the encoding of newlines for UNIX/Windows).

Before submitting this PR, please make sure:

  • You have added a few sentences describing the PR here.
  • You have added yourself or the appropriate individual as the assignee.
  • You have added at least one relevant code reviewer to the PR.
  • Your code builds clean without any errors or warnings.
  • You have added tests (when appropriate).
  • You have updated the README or other documentation to account for these
    changes (when appropriate).
  • You have added an entry to the relevant CHANGELOG.md (see
    "keep a changelog" for more information).
  • Your commit messages follow the conventional commit style.

@claymcleod claymcleod self-assigned this Oct 21, 2024
@claymcleod claymcleod marked this pull request as draft October 21, 2024 01:23
@peterhuene
Copy link
Collaborator

I ended up implementing some of this in #231 as I was trying to run tests on Windows during investigating the LSP issue.

I rolled the "test examples" into the "test" job, but haven't updated gauntlet/arena to run on non-linux platforms.

@claymcleod claymcleod force-pushed the ci/multi-platform branch 2 times, most recently from 4a78930 to dc08657 Compare October 21, 2024 18:34
@claymcleod
Copy link
Member Author

@peterhuene are you good with me enabling this on gauntlet and arena? Or do you think we should just ignore that? My gut feeling is that it should be tested.

@peterhuene
Copy link
Collaborator

@claymcleod totally fine with it; certainly can't hurt.

@claymcleod claymcleod force-pushed the ci/multi-platform branch 6 times, most recently from f8e8989 to e5c6e70 Compare November 1, 2024 14:24
This is a combination of a number of commits that were squashed:

ci: makes relevant CI tests multi-platform
fix: fixes the cloning of repositories on Windows
fix: fixes the path resolution for gauntlet on Windows
fix: a number of other small fixes
@claymcleod claymcleod marked this pull request as ready for review November 1, 2024 14:31
@claymcleod claymcleod merged commit c2e042f into main Nov 1, 2024
16 checks passed
@claymcleod claymcleod deleted the ci/multi-platform branch November 1, 2024 14:35
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