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

Update organization of the unit tests #18

Merged
merged 7 commits into from
Oct 7, 2024
Merged

Conversation

nwlandry
Copy link
Collaborator

@nwlandry nwlandry commented Oct 3, 2024

This PR does the following:

  • Organizes the unit tests and files into two categories: HIF-compliant and HIF non-compliant. The unit tests are split into two files and the json files are put into two separate folders according to HIF compliance/non-compliance.
  • Organizes conftest.py and the unit test files roughly in these categories: top-level attributes, metadata, nodes, edges, and incidences.
  • Format the JSON files consistently (And the schema) with VSCode formatter (https://www.geeksforgeeks.org/how-to-format-json-in-vscode/)
  • Adds several unit tests to the test suite
  • Tweaks the README and License files.
  • Tweaks the schema because it was not catching extraneous fields in the incidence records

@nwlandry
Copy link
Collaborator Author

nwlandry commented Oct 3, 2024

Hi @colltoaction and @FraLotito --- would really appreciate your thoughts on these changes! I reorganized the unit tests to make them easier to contribute to.

@FraLotito
Copy link
Collaborator

Thanks @nwlandry! I'll check it out in the following days

Copy link
Collaborator

@colltoaction colltoaction left a comment

Choose a reason for hiding this comment

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

I'm wondering if we can remove some duplication like return json.load... but I don't see any pytest feature to aid with this.

WRT formatting it would be good to understand what this formatter is doing but it's fine with me. An external formatter / linter could be ran in Actions for guardrailing.

Thank you!

Copy link
Collaborator

@FraLotito FraLotito left a comment

Choose a reason for hiding this comment

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

Ok I agree with this structure, I think the changes improve the readability and organization of the tests. Thanks @nwlandry !

@FraLotito FraLotito merged commit 23dc00c into main Oct 7, 2024
2 checks passed
@nwlandry nwlandry deleted the update-test-organization branch October 7, 2024 12:12
@nwlandry
Copy link
Collaborator Author

nwlandry commented Oct 7, 2024

I'm wondering if we can remove some duplication like return json.load... but I don't see any pytest feature to aid with this.

WRT formatting it would be good to understand what this formatter is doing but it's fine with me. An external formatter / linter could be ran in Actions for guardrailing.

Thank you!

Good point! I'll see what's out there.

@nwlandry
Copy link
Collaborator Author

nwlandry commented Oct 7, 2024

@FraLotito and @colltoaction, thanks for reviewing!!

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.

3 participants