Tests, workflow, continuous integration about python-mip HOT 9 CLOSED

Hi @tommyod , in the last version I managed to automate the test of all examples in the examples folder. The automated tests also have been expanded, now we have ~500 tests (when gurobi is also tested) and I'm not commiting direct to master lately (except for trivial/small changes). So I'll close this issue. You can create more specific issues if you think that some point still needs attention. Thanks for your suggestions.

from python-mip.

Examples of example files failing

I tried to run the files in alphabetical order. Here are some of the failures:

• bmcp.py -> assert round(m.objective_value) == 21 fails. The assert assert m.status == OptimizationStatus.OPTIMAL passes, but the reported objective function value is 41.
• cuttingstock_cg.py fails with

Solution: Traceback (most recent call last):
  File "../examples/cuttingstock_cg.py", line 90, in <module>
    print('{v.name} = {v.x:.3}  {v.column}'.format(**locals()))
  File "/home/tommy/Desktop/github/python-mip/mip/entities.py", line 537, in column
    return self.__model.solver.var_get_column(self)
  File "/home/tommy/Desktop/github/python-mip/mip/cbc.py", line 847, in var_get_column
    col.constrs.append(Constr(self, cidx[i]))
AttributeError: 'NoneType' object has no attribute 'append'

• cuttingstock_kantorovich.py does not run, I get:

Solution: Traceback (most recent call last):
  File "../examples/cuttingstock_kantorovich.py", line 38, in <module>
    print('{v.name} = {v.x:.3}'.format(**locals()))
ValueError: Precision not allowed in integer format specifier

from python-mip.

I appreciate all the work that is going into "python-mip" and more generally the coin-or project.

I haven't looked at the code in detail, but from skimming the repo I'm hesitant to use this library, since it seems a bit far removed from best practices regarding testing, working with continuous integration, and so forth. Here's a list of observations that make me a bit nervous:

* _Almost no tests._ There are only 12 tests. Furthermore it seems that commits are being done directly to master, so there is no guarantee that the master is in a state where tests pass.

* _Examples not working._ There's a folder with examples, but many of these fail when I try to run them (or run for a long time). The examples could be programmatically run as smoketests by Travis, ensuring that documented examples always work.

* _Code style._ Most projects have some code style enforced by Travis or some other CI service, e.g. "black" or "flake8". This makes it easier to enter a project.

* _Committing to master._ This is mentioned above, but there are commits in the history that are one-word changes. Every change should go through a PR so tests/codestyle/examples/etc can be run. Single-word commits can be squashed to a larger commit, so every commit in the main history is meaningful (as opposed to hundreds of very small changes).

I can help set up tests, code style, running examples, etc - but it's of not use if commits are performed directly to the master branch (since it bypasses Travis). Thoughts on the above? Want some help?

Thank you for your detailed analysis @tommyod . I agree with all your points. Help is much needed, yes ! Recently @pmla improved our tests (#28).

We have continuous integration with Travis so that after every commit tests are executed to see if everything is ok. It would be better if a separate branch could accommodate these commits (and test it) before sending directly to master.

Regarding the examples, I think we tested all examples that are in the documentation. Some examples are still incomplete but are kept in the repository in the hope that some good soul could finish, document them and include in the documentation.

from python-mip.

Great. I apologize if my previous post came off as too critical. I believe the effort is very worthwhile. I can't make any promises with regard to involvement, but I'll see if I can help out a bit.

It would be better if a separate branch could accommodate these commits (and test it) before sending directly to master.

Exactly. Everyone should work in feature branches. This includes library authors such as yourself. That way the master branch is always in a good state, and you can squash commits before merging (to make the history more meaningful and shorter).

I think we tested all examples that are in the documentation.

They might have been tested in the sense that they were run at some point in time, but how do you ensure the are up to date with recent developments? E.g. will they run 2 years from now? If the code is changed, will you remember to update the examples? A solution too all these issues is to add them as tests (unless they are huge and take a long time to run).

from python-mip.

Another think, right now in the travis.yml the operating system is not specified and the tests run all in linux. Could we specify that the tests should run also in MacOSX and Windows 10 ?

from python-mip.

I see that you figured out OSX already. If you want to run on Windows, sign up for appveyor and add a minimal .appveyor.yml file. I can set up testing for Windows, but I cannot sign up for Appveyor since I'm not a repo owner.

Please start using feature branches. This is how I typically work on my own projects:

$ git merge origin master --ff-only # update master locally
$ git checkout -b my_branch # create a new feature branch and switch to it
$ <changes>
$ git commit -am "changes"
$ <changes>
$ git commit -am "more changes"
$ git push origin -u my_branch # Push the feature branch
<create PR, let CI run, squash-merge the feature branch on GH>
$ git checkout master
$ git branch -d my_branch
$ git merge origin master --ff-only # update master locally

This way you can:

• squash merge commits if you like. This makes each commit meaningful.
• let CI run BEFORE you merge into master. If you rely on CI running AFTER merging to master, the master can easily be in a failed state. If others merge the master at this time, your erroneous code is propagated to others. This is bad practice, and following best practice is so easy with feature branches.

from python-mip.

Hi @tommyod , I tried to setup appveyour but I'm failing at some point, it seems that it is always using python 2, could you take a look ?

https://ci.appveyor.com/project/h-g-s/python-mip/build/job/ero8vvmv5t3fpo4r

from python-mip.

@tommyod , managed to get appveyour running ! testing with python 3.5 - 3.7

from python-mip.

Fantastic. Keep up the great work!

from python-mip.