search

FahrplanDatenGarten / pyhafas

A python client for the HAFAS (HaCon Fahrplan Auskunfts System)
https://pyhafas.readthedocs.io
MIT License
45 stars 18 forks source link

feat: add automated tests #24

Open akloeckner opened 1 year ago

akloeckner commented 1 year ago

I thought, I'd try and contribute even if it's just a triviality. So, I configured and tested a GitHub action to run tests on changes in main and weekly. See #20 .

The action was proposed by GitHub. It might not be appropriate. So, please just close, if that is the case.

I'm especially unsure about the tox environments. They seem to have a matrix on dependencies which I don't know how to reproduce in GitHub actions. Also, tox seems to test against py36, which is no longer supported by the GitHub python setup action. (It's also outdated. So, maybe drop it?)

1Maxnet1 commented 1 year ago

Additionally to get them into sync, I would propose to update the python versions in tox.ini accordingly.

akloeckner commented 1 year ago

only execute local tests

Does that mean, exclude the tests in the request folders?

1Maxnet1 commented 1 year ago

only execute local tests

Does that mean, exclude the tests in the request folders?

Yep. These should definitely pass at any time.

akloeckner commented 1 year ago

I updated the Python versions to 3.8 trough 3.11.

I also removed the weekly trigger, but kept the remote API access tests, because:

  1. I think,those remote API things should be tested for each PR. If those tests fail occasionally, they can be retriggered.
  2. Since the API accesses are not tested each week anymore, they will not bother us unexpectedly.
  3. It is important to test the actual API access for each PR.
  4. I dont see the use to re-test the local tests weekly. The only reason for them to fail could be faulty, new Python core code. And this should not really be happening much.

What do you think?

1Maxnet1 commented 1 year ago

I updated the Python versions to 3.8 trough 3.11.

I also removed the weekly trigger, but kept the remote API access tests, because:

1. I think,those remote API things should be tested for each PR. If those tests fail occasionally, they can be retriggered.

2. Since the API accesses are not tested each week anymore, they will not bother us unexpectedly.

3. It is important to test the actual API access for each PR.

4. I dont see the use to re-test the local tests weekly. The only reason for them to fail could be faulty, new Python core code. And this should not really be happening much.

What do you think?

In my opinion this is a good solution for the problem :+1:

akloeckner commented 1 year ago

Great! One more thing, I'd like to check is whether we can use tox itself for the automated tests. I tried at first but had some difficulty related to Python versions. May be it will work now.

akloeckner commented 1 year ago

I think, I got the GitHub actions working with tox. The issues were actually with Python 3.10 compatibility in the dependencies. After all, spotting those things, that's what continuous tests are for...

So, I updated the dependencies to requests>=2.28 and pytz>=2022.1 .

See also https://github.com/psf/requests/commit/8bce583b9547c7b82d44c8e97f37cf9a16cbe758 and https://github.com/stub42/pytz/commit/2ed682a7c4079042f50975970fc4f503c8450058 .

Sorry for the messy commit log.