search

ilpincy / argos3

A parallel, multi-engine simulator for heterogeneous swarm robotics
http://www.argos-sim.info/
268 stars 121 forks source link

Remove incomplete code and revamp testing infrastructure #185

Closed allsey87 closed 3 years ago

jharwell commented 3 years ago

I think this would be a good thing to pair to with #159, if @ilpincy has the time to set up the travis integration. I just opened a pull request with the necessary config updates (#186).

allsey87 commented 3 years ago

Personally, I lean more towards using Github Actions since this way we keep everything on the same platform

allsey87 commented 3 years ago

@jharwell could you have a look at this latest commit and let me know if you are ok with this (the Github Actions-based) solution instead from Travis CI?

jharwell commented 3 years ago

I agree that keeping everything on one platform makes sense if feasible, which it seems to be. My main concern would be that the current version you have doesn't have the same coverage in terms of the build matrix (compiler version vs. OS). ALso, there isn't any testing on OpenSUSE, which was something that @ilpincy had as a requirement (see #159 for the discussion).

However, it may be that perfect is the enemy of good enough in this case and it is more important to get some testing framework up and running rather than trying to get the most comprehensive framework defined right out of the gate.

allsey87 commented 3 years ago

My main concern would be that the current version you have doesn't have the same coverage in terms of the build matrix (compiler version vs. OS)

I see where you are coming from but I also think that we need to start being clear about what exactly we want to test. For example, compilation with different versions of dependencies vs. the correctness of the code. My thoughts on the matter are as follows:

  1. In my experience, most issues that come up with ARGoS are related to the correctness of the code and not its compatibility with different versions of packages. As an example, determinism and proper implementations of IsColidingWithSomething and CheckIntersectionWithRay for each physics engine tend to be a far more common source of bugs
  2. I think we should use the limited time that we have to write a broader range of tests rather than running fewer tests on every combination of OS, compiler, and dependency version.
  3. I don't think testing N versions of GCC/CLang is necessary -- compilation under the default version that is provided with an updated OS should be sufficient -- if someone is using a custom toolchain then that's on them
  4. By compiling on two LTS releases of Ubuntu (2 years apart by definition) and two versions of MacOS, we already have quite broad coverage and it is highly likely that, if all tests pass with this coverage, ARGoS will build on any version of Linux released in the past couple of years

However, it may be that perfect is the enemy of good enough in this case and it is more important to get some testing framework up and running rather than trying to get the most comprehensive framework defined right out of the gate.

I agree with this point. ARGoS needs proper testing as soon as possible.

ilpincy commented 3 years ago

Regarding testing, my main concern is adding extra dependencies on software I don't currently master, and the need to maintain it.

I agree we need more automatic testing, but I don't have the bandwidth to study in depth how to integrate ARGoS with new platforms.

I am afraid that some change in these platforms will break stuff, and that will explode in my face (it happens every 18 months with Qt and every ~3 years with CMake, but the drawbacks overcome the benefits).

@jharwell: if you agree to be responsible for these things medium-term (say for the next 2-3 years), then I'd be happy to give you admin access and make the necessary changes. Being responsible means that, if something breaks, you'd be willing to work on it in a reasonable time (say within a couple of weeks for minor issues, or days if it's a catastrophic error).

allsey87 commented 3 years ago

Based on the discussion above, I propose that I merge this PR and that we give Github Actions a try. If it doesn't meet our needs, we can set up something more complicated with Travis CI etc. at a later point in time. Let me know if there are any other issues or concerns, otherwise I will merge this PR in 24 hours from now.

ilpincy commented 3 years ago

Fine with me, go ahead with the PR. Thanks for the work!