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

Error running test #46

Open
TomNicholas opened this issue Aug 21, 2024 · 4 comments
Open

Error running test #46

TomNicholas opened this issue Aug 21, 2024 · 4 comments
Labels
bug Something isn't working testing

Comments

@TomNicholas
Copy link
Contributor

Traceback (most recent call last):
  File "/Users/tom/Documents/Work/Code/C-Star/tests/test_roms_marbl_example.py", line 56, in <module>
    roms_marbl_local_case.setup()
  File "/Users/tom/Documents/Work/Code/C-Star/cstar/case.py", line 711, in setup
    inp.get(self.caseroot)
  File "/Users/tom/Documents/Work/Code/C-Star/cstar/base/input_dataset.py", line 131, in get
    raise FileExistsError(
FileExistsError: A file by the name of roms_grd.ncalready exists at /Users/tom/Documents/Work/Code/C-Star/roms_marbl_local_case/input_datasets/ROMS/.

I ran the tests again locally and got an error. I think this is related to both using pytest #42 (as we can use pytest tmpdir to clean up after our tests run), and also potentially #17 (functions should perhaps be designed so that they don't care if they have already been run before).

@TomNicholas TomNicholas added bug Something isn't working testing labels Aug 21, 2024
@dafyddstephenson
Copy link
Contributor

Yes, related to pytest, not 17 though - this error is triggered by test_roms_marbl_example.py attempting to move files around rather than anything internal to C-Star. I could add something at the start of the test to just delete any files created by previous runs?

@TomNicholas
Copy link
Contributor Author

not 17 though

Are you sure? It literally happens when we call .setup().

test_roms_marbl_example.py attempting to move files around rather than anything internal to C-Star.

Why does it move files around? Why does it do anything outside of C-Star?

I could add something at the start of the test to just delete any files created by previous runs?

pytest's tmdir fixture can be used to automatically delete any files created during the running of that test, which is neater (cleaning up immediately is neater than cleaning up only on the next time the test runs).

@TomNicholas
Copy link
Contributor Author

TomNicholas commented Aug 21, 2024

In fact we may want to think more broadly about structuring the building/running of cstar in such a way that all files created are only created in new directories (which can be temporary directories, to make cleaning up easier).

@dafyddstephenson
Copy link
Contributor

dafyddstephenson commented Aug 22, 2024

Are you sure? It literally happens when we call .setup().

Ah yes, this isn't what I meant. I can't actually reproduce this, for me setup() correctly applies check_exists_locally to the file in question, doesn't call get(), and so never reaches this point in the code (which is designed as much as possible in this regard not to repeat costly methods like obtaining files when get() has already been called/isn't needed). I can run the section of the script in question over and over without problem. Can you write a script with a minimal working example of the error?

Why does it move files around? Why does it do anything outside of C-Star?

The script was expanded to include a new section to test support for local paths to input datasets. It currently:
In C-Star:

  • Runs the example case and dumps the blueprint to a yaml file (test_blueprint.yaml).

Outside of C-Star:

  • Moves all the files downloaded by the example case to a new directory, simulating the situation where they exist locally on the system somewhere independent of C-Star
  • Copies test_blueprint.yaml to modified_test_blueprint.yaml and replaces any URLs in the file with these local paths

Back in C-Star:

  • Creates a new case from this modified blueprint and runs it.

The "Outside C-Star" bit sort of has to exist as on a github runner we have to set up the situation where input datasets exist "elsewhere" on the system. It could be shell scripted, but I knew the whole test process was getting overhauled in the near future anyway and found having a single self-contained python script cleaner and more convenient for personal use. Ditto the string replacement in the yaml - automating this meant:

  • avoiding creating a separate blueprint file with trivial differences (URLs vs local paths) and keeping both in sync
  • testing the ability of C-Star to create a Case from a blueprint file created by C-Star.

I have now added some lines in test_roms_marbl_example.py (#43) that delete the output of previous runs. tmpdir would be a useful alternative once we switch to pytest. I won't close this in case you are still able to produce this issue though.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working testing
Projects
None yet
Development

No branches or pull requests

2 participants