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

Refactor tests/convert tests #1025

Closed
wants to merge 6 commits into from

Conversation

emiliom
Copy link
Collaborator

@emiliom emiliom commented Apr 6, 2023

This is the work I did on 2022-12-22 during the echopype "hack day". At the time I couldn't merge it until #902 was merged, but I didn't realize that happened last December. I've updated it to work with the current dev.

Creating a PR now before my changes diverge too much from dev and become harder to merge.

  • Moved code and fixtures from individual test modules into the new tests/convert/conftest.py module.
  • Focused mainly on files and file paths.
  • Also focused on already existing fixtures, even if they're not file paths.
  • Turned file-path-handling existing code into new fixtures typically if it involved more than one file path.
  • One goal was to divide tests into integration vs unit test modules. I didn't address unit tests, but I renamed modules to *_integration.py when appropriate.
  • Created new convert_time_encodings_params fixture in convert/conftest.py to support test_convert_source_target_locs_integration.py
  • Simplified many tests to remove unnecessary conversion of file paths to str and/or rely more directly on pathlib.

DON'T REVIEW YET! I'm realizing that I may not have included all the changes I made.

emiliom added 6 commits April 6, 2023 09:40
…e with test_path and relying more directly on pathlib paths
…e there. Rename and simplify azfp convert tests, replacing azfp_path fixture with test_path, relying more directly on pathlib.
… fixture. Rename ek80 convert module and simplify tests, replacing ek80_path fixture with test_path and relying more directly on pathlib.
…d add new convert_time_encodings_params fixture for it. Rename source_target_locs convert module and simplify tests to rely more directly on pathlib (remove conversion to str).
@emiliom emiliom added the tests label Apr 6, 2023
@emiliom emiliom self-assigned this Apr 6, 2023
@emiliom emiliom marked this pull request as draft April 6, 2023 18:20
@leewujung
Copy link
Member

I'll close this PR now, since this is very outdated and many tests have changed. We'll pick up the rest of tests structural overhaul in a later PR.

@leewujung leewujung closed this Feb 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

2 participants