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

Fix ci testing #72

Closed
wants to merge 8 commits into from
Closed

Fix ci testing #72

wants to merge 8 commits into from

Conversation

Mews
Copy link
Contributor

@Mews Mews commented Jun 11, 2024

Pull Request

Description

The workflow which was being used to run the tests and create the coverage report was causing the issue described in #71, as it was attempting to run sudo apt-get update || true, which is a linux command, on jobs running macos.

To fix this, I have made the following changes:

  • Created a new workflow from scratch which has the same functionality as the original workflow.
    The new workflow does the following:
    • Define a matrix of jobs for all the provided python versions and os's to test;
    • Sets up a python installation with the job's designated python version;
    • Installs the project dependencies and the dependencies required to run the tests (which are stored in a new file in tests/requirements.txt);
    • Runs the tests with pytest and the extensions xdist and pytest-cov, which were both also used in the original workflow;
      • The tests are ran using a certain number of cpu cores, defined in the pytest_cpus environment variable inside the workflow file;
      • The directory which is checked for the coverage report is defined in the pytest_cov_dir environment variable inside the workflow file;
    • Shows a coverage report in the job's console;
    • Uploads a coverage report to Codecov;
      • I've added this step because it was also present in the original workflow and so I assumed there was some sort of configuration for this already in place. However, if this isn't the case, it might be best to remove this step;
  • Created a new file in test/requirements.txt where the requirements for running the tests are stored, these requirements being pytest, the used extensions pytest-xdist and pytest-cov and coverage;
  • Added code to the beginning of the test scripts to add the path to the source to sys.path;
    • This is required so that you can actually do import dgmr to run the tests;

How Has This Been Tested?

I've tested this action on my fork of the repository.
If you navigate to the actions page you can see that the action runs successfully and that the tests are run correctly.

Checklist:

  • My code follows OCF's coding style guidelines
  • I have performed a self-review of my own code
  • I have made corresponding changes to the documentation
  • I have added tests that prove my fix is effective or that my feature works
  • I have checked my code and corrected any misspellings

Mews added 8 commits June 11, 2024 20:53
This step runs the pytest command to run the tests using the extensions xdist and pytest-cov
In these steps a coverage report is logged in the job's console, and the codecov/codecov-action@v2 action is used to upload the coverage report to Codecov.

This step is based on the originally used workflow https://github.com/openclimatefix/.github/blob/main/.github/workflows/python-test.yml, some sort of codecov key might need to be configured
This is needed so that pytest can actually import dgmr to run the tests
@Mews Mews closed this Jun 18, 2024
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.

Fix CI testing
1 participant