Remove some agasc supplement bad star tests

jeanconn commented 3 months ago

Description

Remove some agasc supplement bad star tests .

This does not address the issue that maybe this test_agasc_supplement should be removed or updated to work with a static or versioned supplement.

Fixes issue that the asserts in the test were not valid after removing some stars from the bad list.

Interface impacts

Testing

Unit tests

[x] Mac arm64


(ska3) flame:proseco jean$ git rev-parse head
a25c3a78e4321590f33082062b2d461607dfbb5e
(ska3) flame:proseco jean$ pytest 
========================================================================================== test session starts ==========================================================================================
platform darwin -- Python 3.11.8, pytest-8.0.2, pluggy-1.4.0
rootdir: /Users/jean/git
configfile: pytest.ini
plugins: timeout-2.2.0, anyio-4.3.0
collected 167 items

proseco/tests/test_acq.py ..................................... [ 22%] proseco/tests/test_catalog.py .......................................... [ 47%] proseco/tests/test_core.py ........................... [ 63%] proseco/tests/test_diff.py ...... [ 67%] proseco/tests/test_fid.py ............... [ 76%] proseco/tests/test_guide.py .................................... [ 97%] proseco/tests/test_mon_full_cat.py .... [100%]

=========================================================================================== warnings summary ============================================================================================ ../../miniconda_m1/envs/ska3/lib/python3.11/site-packages/tables/node.py:251: 1 warning proseco/proseco/tests/test_acq.py: 35 warnings proseco/proseco/tests/test_catalog.py: 52 warnings proseco/proseco/tests/test_core.py: 17 warnings proseco/proseco/tests/test_fid.py: 18 warnings proseco/proseco/tests/test_guide.py: 109 warnings proseco/proseco/tests/test_mon_full_cat.py: 5 warnings /Users/jean/miniconda_m1/envs/ska3/lib/python3.11/site-packages/tables/node.py:251: DeprecationWarning: alltrue is deprecated as of NumPy 1.25.0, and will be removed in NumPy 2.0. Please use all instead. self._v_objectid = self._g_open()

-- Docs: https://docs.pytest.org/en/stable/how-to/capture-warnings.html ================================================================================== 167 passed, 237 warnings in 21.53s



Independent check of unit tests by [REVIEWER NAME]
- [ ] [PLATFORM]:

### Functional tests
<!-- Describe and document results of any functional tests, otherwise leave the text below -->
No functional testing.

javierggt commented 3 months ago

I would just remove the test. That test does nothing here, and the test itself is trivial. It does not even test something that conceivably proseco would depend on.

jeanconn commented 3 months ago

"It does not even test something that conceivably proseco would depend on". well - it tests that there is a bad_star_set loaded from a supplement and it has some stars in it.

javierggt commented 3 months ago

The whole point of unit testing is that each package is tested on its own, and other packages that depend on them do not have to repeat the tests. Then, when the package changes, the tests change accordingly. When tests are replicated in other places, then one has to go and fix them (like now).

I can imagine a case where one adds a test of a dependency as a kind of precondition, so when other tests fail it is clear that there is a root cause. This is not that case. This is an unnecessary test that is already causing us to make this PR that should have never happened.

jeanconn commented 3 months ago

My point was that this is actually a test that provides coverage of the loading of the bad star list from proseco's little wrapper https://github.com/sot/proseco/blob/master/proseco/characteristics.py#L78 .

jeanconn commented 3 months ago

But I suppose that gets coverage from almost every other test, just no confirmation that stars were actually loaded.

sot / proseco