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: split test contact.py #369

Merged
merged 19 commits into from
Feb 28, 2023
Merged

Conversation

joyceljy
Copy link
Collaborator

@joyceljy joyceljy commented Feb 23, 2023

I split the original test in features/test_contact.py into four smaller tests.
They were test_vanderwaals(), test_attractive_electrostatic(), test_repulsive_electrostatic() and test_cal_residue_contact().

@joyceljy joyceljy changed the title feat: split test contact.py joyceljy test: split test contact.py joyceljy Feb 23, 2023
@joyceljy joyceljy requested a review from DaniBodor February 23, 2023 13:40
@joyceljy joyceljy changed the title test: split test contact.py joyceljy test: split test contact.py Feb 23, 2023
Copy link
Collaborator

@DaniBodor DaniBodor left a comment

Choose a reason for hiding this comment

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

Thanks for picking this up, Joyce! It is much more clearly organized into groups the way you now split the tests up.
Ideally, you could split them up even further into individual tests. Pretty much each time there's a comment block saying what is being tested, it could be a single test function. You can then use what is now in the comment as a docstring for that test.

Because most of the tests will be essentially similar, where mainly the inputs and assert statements will change, you can create a sort of helper function that contains the main body of the test, and then call this function with a different input each time.

Also, don't forget to tag which issue you will be closing with this PR ;)

Let me know (on Teams) if it's unclear what I mean with this or if you need help, and I can explain it better.

@joyceljy joyceljy linked an issue Feb 24, 2023 that may be closed by this pull request
@joyceljy
Copy link
Collaborator Author

Update:
I split the function into smaller test cases(based on the original comments). And also changed the comments into docstring format:) For the duplicate part appearing in each test cases, I defined a help function _wrap_pdb_get_atomiccontact, which assigned the pdb_path and called the AtomicContact object. The parameters (residue_number & atom_name )needed for the AtomicContact object are passed from each test case.

@joyceljy joyceljy requested a review from DaniBodor February 24, 2023 13:55
Copy link
Collaborator

@DaniBodor DaniBodor left a comment

Choose a reason for hiding this comment

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

you can go ahead and merge :)

@joyceljy joyceljy merged commit c6d2f4a into main Feb 28, 2023
@joyceljy joyceljy deleted the 367_Split_test_contact.py_joyceljy branch February 28, 2023 12:00
@gcroci2 gcroci2 changed the title test: split test contact.py refactor: split test contact.py Mar 3, 2023
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.

Split tests in tests/features/test_contact.py into separate functions
2 participants