Update integration tests to use pytest conventions

[Sherwin-14]

Integration tests have been failing silently for some time (#738). In the course of adding new integration tests (#736), we found some errors that were not reporting very much information, presumably either because the code that's generating the errors are not following pytest conventions.

1. Tests which import assertions and call for example assertions.assertTrue(x) should be updated to builtin assertions, for example assert x is True.
2. Tests which do setup at the module level should be updated to use fixtures

[Sherwin-14]

Hey, Could you assign me this one? I have started to work on this and I had some doubts. First of all, I have updated all the tests having assertions.assertTrue(x) to assert x is true. So this is exactly what we want here isn't it? Secondly I went through all the test files and wasn't able to find the module level setups without fixtures. Can you highlight some specific ones so that I get a hang of what I need to do here?

[mfisher87]

This test file is a good example:

earthaccess/tests/integration/test_onprem_download.py

Lines 51 to 63 in 2df1da9

	assertions = unittest.TestCase("__init__")
	# we need to use a valid EDL credential
	assertions.assertTrue("EARTHDATA_USERNAME" in os.environ)
	assertions.assertTrue("EARTHDATA_PASSWORD" in os.environ)
	auth = Auth().login(strategy="environment")
	assertions.assertTrue(auth.authenticated)
	logger.info(f"Current username: {os.environ['EARTHDATA_USERNAME']}")
	logger.info(f"earthaccess version: {earthaccess.__version__}")
	store = Store(auth)

Looks like most of the integration test files have this problem, but not all (e.g. test_auth.py looks OK).

[Sherwin-14]

This test file is a good example: https://github.com/nsidc/earthaccess/blob/main/tests/integration/test_onprem_download.py#L51-L63 Looks like most of the integration test files have this problem, but not all (e.g. test_auth.py looks OK).

Yeah but I guess the granules function here is used as a setup and it has already been fixturized(If that's a term). Also this means that we juts need to update codeblocks like the above example, and not to tinker with the other assertions.assertTrue(x) expressions.

[mfisher87]

I wouldn't describe get_sample_granules or the auth store setup as "fixturized" at the moment, at least under Pytest conventions, since they're not using a fixture decorator (and the auth stuff is bare at the module level). Have you checked out this doc yet? https://docs.pytest.org/en/6.2.x/fixture.html

The pytest fixture decorators provide advanced functionality for managing the fixture's lifecycle or scope. That way, we don't need to repeat behavior unnecessarily (we do this same credential-reading stuff at the module level in several files in addition to test_onprem_download.py, but we only need to do it once:

earthaccess/tests/integration/test_onprem_open.py

Lines 50 to 62 in 2df1da9

	assertions = unittest.TestCase("__init__")
	# we need to use a valid EDL credential
	assertions.assertTrue("EARTHDATA_USERNAME" in os.environ)
	assertions.assertTrue("EARTHDATA_PASSWORD" in os.environ)
	auth = Auth().login(strategy="environment")
	assertions.assertTrue(auth.authenticated)
	logger.info(f"Current username: {os.environ['EARTHDATA_USERNAME']}")
	logger.info(f"earthaccess version: {earthaccess.__version__}")
	store = Store(auth)

), and we get better reporting from pytest to help us understand what's going on :)

[Sherwin-14]

Oh there seems to be some misunderstanding here! I was talking about the granules function in test_kerchunk.py that is fixturized at the moment. So I was basically asking for one more example of a setup process that is not fixturized uptill now.

[mfisher87]

Gotcha, thanks for clarifying! Just double-checking, do you have the info you need now?

[Sherwin-14]

Not yet 😅. I just need an example of the setup function that is not fixturized yet.

[mfisher87]

In this case, it's not a function, it's the module-level code I linked above which creates an Auth and Store object. Does that make sense?

[Sherwin-14]

The auth and store objects can be considered as setup's, is that what you are implying? Also I think that the conftest.py file should contain all the fixturized functions. So I am thinking of putting all the fixturized functions in that file, would that be fine with you?

[mfisher87]

The auth and store objects can be considered as setup's, is that what you are implying?

Correct!

Also I think that the conftest.py file should contain all the fixturized functions. So I am thinking of putting all the fixturized functions in that file, would that be fine with you?

Sounds great :) Thanks again @Sherwin-14 for your work!

[Sherwin-14]

Hey, In that case may be grouping them into a function and then fixturizing it can be a solution isn't it? or Am I missing something?

[mfisher87]

Yes, exactly :) I don't think you're missing anything! I think we want scope="module" for the new fixture.