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

Deprecate PymatgenTest, migrate pymatgen tests to pytest from unittest #4212

Open
wants to merge 33 commits into
base: master
Choose a base branch
from

Conversation

DanielYang59
Copy link
Contributor

@DanielYang59 DanielYang59 commented Dec 1, 2024

Summary

Warning

  • [NEED CONFIRM] Deprecate PymatgenTest with MatSciTest, set deadline at 2026-01-01

setUp -> setup_method even when not inheriting from TestCase

Test classes that doesn't subclass TestCase and define setUp method would work in pytest, but it's not a native support, so I would still migrate them here:

One thing that might catch users by surprise is that plain setup and teardown methods are not pytest native, they are in fact part of the nose support.
Native pytest support uses setup_method and teardown_method (see Method and function level setup/teardown), so the above should be changed to:

def test_specie_potential(self):
pass

@unittest.expectedFailure
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I might remove this test as it is hard-coded to check the content of a non-existent file:

@unittest.expectedFailure
def test_library_line_explicit_path(self):
gin = self.gio.library_line("/Users/mbkumar/Research/Defects/GulpExe/Libraries/catlow.lib")
assert "lib" in gin

@DanielYang59 DanielYang59 force-pushed the real-real-real-migrate-pmg-tests-to-pytest branch from 9c5bc6d to 344a872 Compare December 3, 2024 14:37
@DanielYang59 DanielYang59 force-pushed the real-real-real-migrate-pmg-tests-to-pytest branch from 3ffea5d to ab8bb0e Compare December 4, 2024 14:47
@DanielYang59 DanielYang59 force-pushed the real-real-real-migrate-pmg-tests-to-pytest branch from 9595a2a to 9ae0deb Compare December 4, 2024 15:32
@DanielYang59 DanielYang59 force-pushed the real-real-real-migrate-pmg-tests-to-pytest branch from 95a3e90 to 42f9fb6 Compare December 4, 2024 15:34
@@ -35,8 +35,15 @@
FAKE_POTCAR_DIR = f"{VASP_IN_DIR}/fake_potcars"


class PymatgenTest(TestCase):
"""Extends unittest.TestCase with several assert methods for array and str comparison."""
class MatSciTest:
Copy link
Contributor Author

@DanielYang59 DanielYang59 Dec 4, 2024

Choose a reason for hiding this comment

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

I originally didn't plan to deprecate PymatgenTest in this PR, but it turns out we have to do this here, otherwise tests that subclassing PymatgenTest would still rely on the TestCase and pytest methods like setup_method/class would not be compatible, meaning we have to do another migration after PymatgenTest dropped TestCase inheritance.

So I guess the easiest way is not to drop inheriting TestCase by PymatgenTest, but replacing PymatgenTest with another one that doesn't subclassing TestCase.

I drafted a name here but free feel to comment if you have better ideas (batch rename would be super easy with an IDE so don't worry).

@DanielYang59 DanielYang59 changed the title Migrate pymatgen tests to pytest from unittest Deprecate PymatgenTest, migrate pymatgen tests to pytest from unittest Dec 4, 2024
@DanielYang59 DanielYang59 force-pushed the real-real-real-migrate-pmg-tests-to-pytest branch from 083879a to 73328d0 Compare December 5, 2024 02:47
@DanielYang59 DanielYang59 marked this pull request as ready for review December 5, 2024 06:16
@DanielYang59 DanielYang59 marked this pull request as draft December 11, 2024 15:05
@DanielYang59 DanielYang59 force-pushed the real-real-real-migrate-pmg-tests-to-pytest branch from fa8ebef to 653714e Compare December 12, 2024 05:06
@DanielYang59 DanielYang59 reopened this Dec 12, 2024
@DanielYang59 DanielYang59 force-pushed the real-real-real-migrate-pmg-tests-to-pytest branch from fa8ebef to 653714e Compare December 12, 2024 05:11
@DanielYang59 DanielYang59 reopened this Dec 12, 2024
@DanielYang59 DanielYang59 force-pushed the real-real-real-migrate-pmg-tests-to-pytest branch from fa8ebef to 58dc537 Compare December 12, 2024 05:15
@DanielYang59 DanielYang59 force-pushed the real-real-real-migrate-pmg-tests-to-pytest branch from ee603fa to 71dd429 Compare December 12, 2024 05:41
@DanielYang59 DanielYang59 force-pushed the real-real-real-migrate-pmg-tests-to-pytest branch from 71dd429 to def9886 Compare December 12, 2024 05:42
@DanielYang59 DanielYang59 marked this pull request as ready for review December 12, 2024 05:52
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.

[Dev/Test] Migrate PymatgenTest from unittest.TestCase to pytest
1 participant