EnergySystemsModellingLab / MUSE_OS

Welcome to the MUSE-OS repository
https://muse-os.readthedocs.io/en/latest/
GNU General Public License v3.0
22 stars 9 forks source link

Tests fail when running the full testing suite #365

Closed HarmonicReflux closed 2 months ago

HarmonicReflux commented 3 months ago

Description

This pull request refactors the way muse --model -default is executed to ensure that all tests pass.

Specifically, the Results/MCACapacity.csv file is now placed within the docs folder, which is where the test expects to find MCACapacity.csv. The necessary file is created during the test run by the notebook itself and subsequently picked up by it. This change resolves the error that causes pytest to fail, and as a result, all tests now pass by default.

Fixes #261

Type of change

Please add a line in the relevant section of CHANGELOG.md to document the change (include PR #) - note reverse order of PR #s.

Key checklist

Further checks

codecov[bot] commented 3 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 71.39%. Comparing base (2e8e1b5) to head (297e8db).

Additional details and impacted files ```diff @@ Coverage Diff @@ ## develop #365 +/- ## =========================================== + Coverage 71.36% 71.39% +0.03% =========================================== Files 44 44 Lines 5915 5915 Branches 1162 1162 =========================================== + Hits 4221 4223 +2 + Misses 1371 1370 -1 + Partials 323 322 -1 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

HarmonicReflux commented 3 months ago

Well, for "Results/MCACapacity.csv" to be visible, one has to create it first, and the most simple way of creating it is by running a model, say the default model via muse --model default. Hence, I included invocation of this command in the documentation. If users then run the test suite, the failure will not occur.

As @dalonsoa pointed out: If my suggestion is not the one to go for, I can think of either: i) Include the file into the codebase (still feels a bit hacky to me) ii) Defining to tun the default model in the setup file such that it is created while running python -m pip install -e .[dev,doc] iii) Any other suggestions?

dalonsoa commented 3 months ago

I think the main issue here is:

  1. to figure out what notebook needs the Results folder in the root folder
  2. then take the steps such that the data it looks for is within the docs directory - like any other of the notebooks - and put the data in the right place.
  3. And yes, committing the data as well, as it is done with the other notebooks that need data, like those of the tutorials.

What makes no sense is to have a notebook in the docs requiring data in the root directory.

There's a discussion going on on how to avoid committing data to the repository, but that's a longer term discussion and we do not have an answer for it, yet.

dalonsoa commented 3 months ago

And remember to update the branch and, ideally, add a PR description - not essential, but serve as reference for the future.

alexdewar commented 3 months ago

Did you want me to review this @HarmonicReflux?

HarmonicReflux commented 3 months ago

Did you want me to review this @HarmonicReflux?

Yes, please take a look, so we are all on the same page. Thks.

HarmonicReflux commented 2 months ago

Appreciated comments of the reviewers.

HarmonicReflux commented 2 months ago

@alexdewar please take a look at this reworked pull request.

tsmbland commented 2 months ago

I'm still a bit unsure about this, because we specifically need a results folder in the docs folder, so running muse --model default in the root folder won't be enough. The link that Alex created in the docs workflow will only exist if the user runs the documentation build locally (right?) which we're not asking people to do before running the tests, so I'm not sure how this fixes the problem. Am I missing something?

HarmonicReflux commented 2 months ago

Running a model will create a "Results" folder in the directory MUSE is installed. The results folder itself then contains subfolder that of which one contains the file the one testing the notebooks needs to have access in order to run successfully. To my knowledge, building the documentation is a separate process unrelated to the tests themselves.

tsmbland commented 2 months ago

I get that, but the notebook in question (running-muse-example.ipynb) points to a Results folder contained in the docs folder, not the root folder

HarmonicReflux commented 2 months ago

@dalonsoa and @tsmbland, please review this pull-request.

@alexdewar is out of office until later this week.