mb/tidy-up #98
mb/tidy-up #98
Conversation
|
I added test classes, so that every recipe is at least somewhat covered, and ironed out everything that was failing (except those that depended on external data). Also changed the |
|
Now this branch should be up to date with I also refactored the way |
| @pytest.fixture(autouse=True, scope="module") | ||
| def reset_edps(): | ||
| def inner(): | ||
| os.system("edps -shutdown") | ||
| os.system("rm -rf /tmp/EDPS_Data/*") | ||
|
|
||
| return inner |
There was a problem hiding this comment.
I'm not so comfortable with having any os.system("rm -rf code in the repository.
There was a problem hiding this comment.
Sure, it's dangerous. Right now I cannot think of a better way of resetting the state though.
There was a problem hiding this comment.
Yesterday I fixed the bug that the EDPS won't start if you delete your EDPS_Data directory, and I created this test:
(Created a fork of the EDPS to highlight that.) There an entire temporary EDPS_Data directory is created, and deleted after the test. Perhaps we can do something similar. I don't fully understand what is going on, I just cargo-culted that test.
(Which, if my fix is accepted, would mean that you would not need that /* in this part of the code.)
Maybe we can keep the code as-is for now. But I just wanted to note here that I don't like it. Which means by extension now I'm also on the line if something ever goes wrong with this code..
.github/workflows/run_edps.yaml
Outdated
| edps -w metis.metis_wkf -i $SOF_DATA -c | ||
| edps -w metis.metis_wkf -i $SOF_DATA | tee edps.stdout.txt |
There was a problem hiding this comment.
It should not be necessary to revert this change right?
There was a problem hiding this comment.
No, and it should be OK now.
|
This branch contains #89 , so should we merge that first? It is hard to disentangle what is going on |
|
Finally I was able to bring it locally to the same state as on GitHub: EDPS has 21 completed, 3 failing. However I still cannot see the actual source of the problem, EDPS logs are not very verbose. |
Added pyesorex tests to CI for all available recipes
| assert output.returncode == 0 | ||
| assert output.stderr == b'' | ||
| assert re.findall('[eE]rror', message) == [] | ||
| assert re.findall('FAILED', message) == [] |
There was a problem hiding this comment.
I don't understand half of this, so I'm in no position to yay/nay it
|
I think we're done here. I self-reviewed all the tests and tried to make them as consistent as possible. |
Tidying up and adding more tests wherever applicable.