Regression tests

[nlehuby]

We need a way to make sure our changes and refactors do not break the current creators.
Maybe a shell script ...

The process could be, for each creator :

• launch osm2gtfs with mocked or persisted OSM input data
• unzip the GTFS output file
• display a diff of all the csv files

[grote]

We definitely need those tests. However, I would prefer these to be in python and not shell scripts. It could be a mix of unit and integration tests.

[Skippern]

one way of testing could be to have a test folder with text files containing expected Overpass output, and load them on test instead of actually calling Overpass

[pantierra]

There has been great progress on this, made by @prhod and @nlehuby. There is a universal structure to include more tests and call them, also in our new CI.

I'm a bit wondering how we will deal with tests that may fail because of changes in the OSM data base. At a first glance, they seem to be problematic, as they might start to fail, even though nothing has changed in code, or the changes don't cause the test to fail. I actually don't have a solution for this, but I think we need to think together to find a good solution.

[grote]

Aren't Overpass responses in those great tests mocked, so they are always the same? This should take care of this problem, right? -- Sent from my Android device with K-9 Mail. Please excuse my brevity.

[nlehuby]

You are right @grote: the OSM data is mocked so we only test the code and not the data.

[pantierra]

Thanks for the clarification. Perfect, that we can mock it up like this.

Regarding tests for creators/cities (like the Accra tests) and tests for core modules, I suggest to follow @jcfausto in his PR #78: We create two subdirectories in tests:

• core - Where tests for the core modules come in.
• creators - Where tests for the different creators/cities live.

The naming convention could be most easily: tests_<SELECTOR>.py for creators, like for example tests_gh_accra.py or tests_ni_managua.py; or in case of core modules it can be tests_<FILENAME>.py, like tests_osm_connector.py or tests_configuration.py.

What do you think?

[prhod]

Hello everyone,
@xamanu : I agree with your proposal (2 directories, and the naming convention)
@grote and @nlehuby : to be precise, OSM Data is mocked for stops and routes, but the search for unnamed stops have been disabled because not necessary for Accra. It should be added, maybe as a core function unit test ?

[pantierra]

The PR on #83 introduces the testing directory structure for core components as discussed in this thread here.

[pantierra]

The tests for Accra have been implemented, they are also on the way for Managua (#131). I guess we can successfully close this issue. What do you think, @nlehuby?

[nlehuby]

That's really good news ! 👍 👍

I think we should have regression tests for all creators in CI though.
But we can close this issue and create specific ones for the incofer and fenix ones.

[pantierra]

Here are tests for all creators: #134

[nlehuby]

Thaks a lot @xamanu.

We should also update the doc to suggest to also write tests for the new creators.

[pantierra]

Yes, I agree @nlehuby, we should document how to create tests for new creators and encourage or maybe even make a condition to implement these before it can be integrated. What do you think?

Updating here on the progress:

• A standard creators test logic has been abstracted: Abstract test classes to share common logic between creators #133 Creators can hook into the code by overriding some functions to provide the data to test against and eventually implement more specific tests.
• I split up the tests for each provider: Add acceptance tests for Florianopolis, Brazil #135, Add acceptance tests for Esteli, Nicaragua #136, Add acceptance tests for Gran Area Metropolitana, Costa Rica #137

There is a discussion in #136 and it also effects #130: Do we want every creator to have tests or can we rely on only one, as long as they are using the standard creator implementation?

[nlehuby]

I agree, we should have all integrated providers tested by CI

[nlehuby]

All creators now have regression tests 🎉 Can we close this issue ?