catalyst-cooperative / pudl

The Public Utility Data Liberation Project provides analysis-ready energy system data to climate advocates, researchers, policymakers, and journalists.
https://catalyst.coop/pudl
MIT License
482 stars 110 forks source link

Convert tests using unittest to pytest #1357

Open bendnorman opened 2 years ago

bendnorman commented 2 years ago

Our codebase currently uses a mixture of pytest and unittest to run tests. In order to standardize, let's convert the remaining instances of unittest into pytest. To do this:

unittest shows up in the following places in our codebase:

$ git grep unittest

test/unit/extract/csv_test.py:from unittest.mock import MagicMock, patch
test/unit/extract/excel_test.py:import unittest
test/unit/extract/excel_test.py:from unittest import mock as mock
test/unit/extract/excel_test.py:class TestMetadata(unittest.TestCase):
test/unit/extract/excel_test.py:class TestExtractor(unittest.TestCase):
test/unit/extract/phmsagas_test.py:from unittest.mock import MagicMock, patch
test/unit/output/ferc1_test.py:import unittest
test/unit/output/ferc1_test.py:class TestForestSetup(unittest.TestCase):
test/unit/transform/classes_test.py:from unittest.mock import MagicMock
test/unit/workspace/datastore_test.py:import unittest
test/unit/workspace/datastore_test.py:class TestDatapackageDescriptor(unittest.TestCase):
test/unit/workspace/datastore_test.py:class TestZenodoFetcher(unittest.TestCase):
test/unit/workspace/resource_cache_test.py:import unittest
test/unit/workspace/resource_cache_test.py:class TestGoogleCloudStorageCache(unittest.TestCase):
test/unit/workspace/resource_cache_test.py:class TestLocalFileCache(unittest.TestCase):
test/unit/workspace/resource_cache_test.py:class TestLayeredCache(unittest.TestCase):
bendnorman commented 2 years ago

@zaneselvans I'm not going to close this issue because some modules in PUDL are still using unittest instead of pytest. What do you think?

zaneselvans commented 2 years ago

Agreed. It would be good to get everything using the same framework.

e-belfer commented 1 month ago

@zaneselvans Is this still relevant, or was this closed in #3296? I'm not seeing unittest in our codebase anywhere. If I've missed something, this needs to be fleshed out to be an actual good first issue.

zaneselvans commented 1 month ago

We do still have a number of cases where we're using unittest, many of which I think could be refactored to use pytest idioms instead. I guess we should identify which ones we actually want to refactor. This might also be something that CoPilot can do pretty effectively...

$ git grep unittest
test/unit/extract/csv_test.py:from unittest.mock import MagicMock, patch
test/unit/extract/excel_test.py:import unittest
test/unit/extract/excel_test.py:from unittest import mock as mock
test/unit/extract/excel_test.py:class TestMetadata(unittest.TestCase):
test/unit/extract/excel_test.py:class TestExtractor(unittest.TestCase):
test/unit/extract/phmsagas_test.py:from unittest.mock import MagicMock, patch
test/unit/output/ferc1_test.py:import unittest
test/unit/output/ferc1_test.py:class TestForestSetup(unittest.TestCase):
test/unit/transform/classes_test.py:from unittest.mock import MagicMock
test/unit/workspace/datastore_test.py:import unittest
test/unit/workspace/datastore_test.py:class TestDatapackageDescriptor(unittest.TestCase):
test/unit/workspace/datastore_test.py:class TestZenodoFetcher(unittest.TestCase):
test/unit/workspace/resource_cache_test.py:import unittest
test/unit/workspace/resource_cache_test.py:class TestGoogleCloudStorageCache(unittest.TestCase):
test/unit/workspace/resource_cache_test.py:class TestLocalFileCache(unittest.TestCase):
test/unit/workspace/resource_cache_test.py:class TestLayeredCache(unittest.TestCase):
e-belfer commented 1 month ago

I updated the issue to clarify what the task actually is!

ggurjar333 commented 4 weeks ago

can I TAKE this?

ggurjar333 commented 4 weeks ago

I plan to implement this using Pytest-mock. Can we use this for replacing unittest logic?

e-belfer commented 2 weeks ago

Hi @ggurjar333, sorry for the slow response! We'll want to switch from importing MagicMock to calling mocker.MagicMock, as we currently do in test_retry() in pudl.test.unit.helpers_test.py.

Looks like you've opened a PR - when you resolve the remaining test failures, we'll go ahead and give it a review. Let us know if you get stuck or have any questions! Thanks for taking this on!

ggurjar333 commented 2 weeks ago

I'm working on this further to solve errors on classes_test.py

Do we have any more context here? @e-belfer

{CB4A75EE-742A-436B-8F6B-28A6818BAC18}
e-belfer commented 2 weeks ago

@ggurjar333 That's a code coverage failure - you can run make pytest-coverage to run all the software tests and the coverage tests in order to check to see whether there are any issues with your PR. See here for more documentation on our testing setup.

zaneselvans commented 2 weeks ago

@ggurjar333 You can also run make pytest-unit which will just run the unit tests without the coverage requirement.

ggurjar333 commented 2 weeks ago

@zaneselvans I think my pytest is not properly getting paths for the root dir or something.

{F26A9FEA-FCAF-430D-A7E7-CC1D101C6125}

I'm not sure what is happening exactly.

{CDD1EA2B-A694-469E-A4DF-52F174892D13}
zaneselvans commented 2 weeks ago

What command are you running and from what directory?