TST: Investigate tox behavior with C-extension

[pllim]

Blocked by: tox 4.x release -- That version will have option to build and test wheel. See tox-dev/tox#852 (comment)

Follow up of #293 . Need to see why C-extension needs to be manually built before running tests in tox. Perhaps it is expected or perhaps it is a symptom of a larger problem. Someone with expertise in tox and C-extension packaging needs to have a look.

synphot_refactor/tox.ini

Lines 15 to 18 in 60be4e9

	# FIXME: Cannot do this, need to force C-ext to build below.
	# Run the tests in a temporary directory to make sure that we don't import
	# package from the source tree
	#changedir = .tmp/{envname}

synphot_refactor/tox.ini

Lines 55 to 59 in 60be4e9

	commands =
	# FIXME: Not sure why need this for tox to see the C-ext
	python -m pip install extension-helpers
	python setup.py build_ext --inplace
	# End of FIXME

[pllim]

Tried many things but nothing worked. @jhunkeler suspected some sort of upstream bug that is out of our control. Maybe when #294 is done, we only have to test the wheels and this problem would be irrelevant?

[jdavies-st]

Need to see why C-extension needs to be manually built before running tests in tox. Perhaps it is expected or perhaps it is a symptom of a larger problem. Someone with expertise in tox and C-extension packaging needs to have a look.

I suspect this is due to this package having a layout of synphot/synphot instead of synphot/src/synphot, which means when you run pytest from the top level either standalone or with tox, it is pulling the code from the repo checkout, not the installed code in synphot/.tox/py39/lib/python3.9/site-packages/synphot. That installed directory has the built C library, but the checkout does not. And this is because tox built the package with pip install .[test] or equivalent, not pip install -e .[test].

A way to get around this is to

1. Move to a synphot/src/synphot layout
2. or, use pytest --pyargs with a changedir directive to some directory that is not the code checkout, ala astropy. This will guarantee that tox is running pytest on the installed code in the tox venv.

Btw, we have the same issue with jwst right now. I can ping you if/when we have a fix. Or you ping us! 😅 @mcara @pllim

There's some discussion of this in

https://docs.pytest.org/en/stable/goodpractices.html

[pllim]

@mcara has an exploratory PR open at #310 . Suggestions or alternate PR welcome! 🙏

[jdavies-st]

Ah, yes! Nice. Yeah, tests should always have absolute imports. Also good.

So many of these pitfalls go away if you package as

synphot/setup.py
synphot/src/synphot/__init__.py
synphot/tests/

Which means the tests are completely separate and have to do absolute imports. And then if you run the tests from the top level there's no ambiguity when a test does import synphot that it needs to do so on the installed version, not the cloned repo. It takes building C extensions to reveal these problems. 😅

If you package the tests separately, you can also chose not to distribute them with the runtime code if you like. And all of a sudden your glob patterns for package_data and everything else become much easier as well. 😂

[pllim]

I am not excited about re-arranging the whole package just so tox would run "properly".

[jdavies-st]

Me neither for jwst. Hence a solution like what astropy uses might be best. Always changedir and always run with pytest --pyargs.

[pllim]

I thought I tried with --pyargs but I really don't remember now.