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

Add builds for windows and macOS #1648

Merged
merged 8 commits into from
Feb 15, 2024
Merged

Add builds for windows and macOS #1648

merged 8 commits into from
Feb 15, 2024

Conversation

Zeitsperre
Copy link
Collaborator

@Zeitsperre Zeitsperre commented Feb 14, 2024

Pull Request Checklist:

  • This PR addresses an already opened issue (for bug fixes / features)
  • Tests for the changes have been added (for bug fixes / features)
    • (If applicable) Documentation has been added / updated (for bug fixes / features)
  • CHANGES.rst has been updated (with summary of main changes)
    • Link to issue (:issue:number) and pull request (:pull:number) has been added

What kind of change does this PR introduce?

  • Adds both a Windows and macOS build to check the testing suite against different platforms.
  • the .lock file generated in the cache is now removed after the testing data has been copied, and the file lock has been released.
  • The prefetch argument for tox is now available for general testing environments.

Does this PR introduce a breaking change?

No.

Other information:

The approved tag is listed here to ensure that the relevant builds are being triggered. This PR still requires review approval.

In order to run Windows build tests, $ xclim prefetch_testing_data must be called first to gather the testing data.

Did you know that Windows doesn't support Unix-style File Locking (fcntl)? This was what was preventing asynchronous handling of testing data fetching from working.

@Zeitsperre Zeitsperre added standards / conventions Suggestions on ways forward approved Approved for additional tests labels Feb 14, 2024
@Zeitsperre Zeitsperre self-assigned this Feb 14, 2024
@Zeitsperre Zeitsperre changed the title add builds for windows and macOS Add builds for windows and macOS Feb 14, 2024
@github-actions github-actions bot added the CI Automation and Contiunous Integration label Feb 14, 2024
Copy link

Note
It appears that this Pull Request modifies the main.yml workflow.

On inspection, the XCLIM_TESTDATA_BRANCH environment variable is set to the most recent tag (v2023.12.14).

No further action is required.

@Zeitsperre Zeitsperre marked this pull request as ready for review February 14, 2024 22:44
@Zeitsperre Zeitsperre requested a review from a team February 14, 2024 22:44
Copy link
Collaborator

@aulemahal aulemahal left a comment

Choose a reason for hiding this comment

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

Nice! Thanks!

I see the test pass, does this mean #1500 can be closed ? (I should read the PR I review)

@Zeitsperre
Copy link
Collaborator Author

Zeitsperre commented Feb 14, 2024

No, you're right, this unfortunately doesn't resolve #1500, but I now know why the windows call to pytest on conda may have been failing previously.

I'm going to try running $ xclim prefetch_testing_data as one of the setup commands prior to testing when we build on conda-forge from now, and that should fix any potential problems from that source.

I'm going to add a call to $ xclim --help here to get more information.

@coveralls
Copy link

coveralls commented Feb 14, 2024

Coverage Status

coverage: 90.171% (+0.01%) from 90.16%
when pulling d27f9a9 on multi-os-builds
into d09bd4b on master.

@Zeitsperre Zeitsperre merged commit 3a049ba into master Feb 15, 2024
21 checks passed
@Zeitsperre Zeitsperre deleted the multi-os-builds branch February 15, 2024 18:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Approved for additional tests CI Automation and Contiunous Integration standards / conventions Suggestions on ways forward
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants