SIMPLE-AstroDB / SIMPLE-db

BSD 3-Clause "New" or "Revised" License
11 stars 21 forks source link

add test for spectrum urls #404

Closed kelle closed 10 months ago

kelle commented 11 months ago

New test to make sure spectrum urls don't give 404. This test takes some time....not sure we want to keep it, but it's useful at the moment. Related to #347.

Link to relevant issue: Closes #369

dr-rodriguez commented 11 months ago

My worry is that this may be attempting to fetch thousands of spectra for each time the tests runs, which is why it takes a longer time.
Perhaps a better approach is that this test exists, but is in a separate file and there is a separate github action for a periodic (once per week maybe) execution as opposed to with every commit.

kelle commented 11 months ago

YES! Here's how to do it: https://docs.github.com/en/actions/using-workflows/events-that-trigger-workflows#schedule

What should we call the file? still test_? If we do that, it will be run by default by pytest. So maybe a different name?

dr-rodriguez commented 11 months ago

We could change the pytest defaults, or just call it something like scheduled_jobs.py or something similar

kelle commented 11 months ago

scheduled_tests?

dr-rodriguez commented 11 months ago

I don't know if the pytest defaults look at anything that starts with test_ or just have test somewhere in it. We probably should explicitly have a pytest.ini file to control that. There's also a way to set markers on pytest so we could have slow/fast tests and configured the current tests to use the fast tests and the scheduled ones to use the slow ones. But if you want to be safe and not add any of that, maybe just avoid test in the filename.

kelle commented 11 months ago

Here's the documentation for pytest autodiscovery: https://docs.pytest.org/en/6.2.x/goodpractices.html#conventions-for-python-test-discovery

My first choice is to make a file/test which just doesn't get auto-discovered but still lives in tests/

kelle commented 11 months ago

This is ready for re-review. It's just the tests. Once it's merged, I'd like to run the action, make sure it works with all the broken URLs.

Then I can work on #409 to actually fix the URLs.