mu-editor / mu

A small, simple editor for beginner Python programmers. Written in Python and Qt5.
http://codewith.mu
GNU General Public License v3.0
1.4k stars 434 forks source link

This was a spike... now needs unit tests. #4

Closed ntoll closed 6 years ago

ntoll commented 8 years ago

Suggestions for testing framework welcome!

NDevox commented 8 years ago

PyQt have their own suggestions:

https://wiki.python.org/moin/PyQt/GUI_Testing

NDevox commented 8 years ago

Most of the above seem to be dead. I'm tempted to go with the pure PyQt/Python framework to build up the tests.

NDevox commented 8 years ago

If I find time I'm probably going to look into building up some tests. Hopefully in the next week or so.

I'm inclined to go down the route suggested above, pure Python and PyQt. But then I'm pretty new to testing interfaces so if anyone else has a better idea please let me know.

ntoll commented 8 years ago

I'm going to start the test suite this evening. I have a pretty good idea what needs to be done and will push the "story so far..." so others can join in.

NDevox commented 8 years ago

I shall wait to follow your lead then :-)

ntoll commented 8 years ago

+1

If I do my job properly it'll be obvious what to do. ;-)

ntoll commented 8 years ago

I've added the start of a test suite in d85461751b0a6426e456a71d47640fd4ddf49b1f

Will finish off (i.e. get to as close as 100% coverage) this evening.

carlosperate commented 8 years ago

That's great! should we start adding these to the travis and appveyor CI scripts?

ntoll commented 8 years ago

Knock yourself out! Put simply make check is what you need.

carlosperate commented 8 years ago

I'm currently getting errors when running on travis. On linux it fails to collect the files (log file), on OS X it shows the following failures: https://travis-ci.org/carlosperate/mu/jobs/103099725#L359

I'll try to look into these later tonight, any clues in the meantime?

ntoll commented 8 years ago

I see the problem. It's me being an idiot. Give me 20 mins and I'll submit a fix.

carlosperate commented 8 years ago

It's currently running on my fork, so any updates to this repository will not trigger the unit test, but I can merge them in and try again (a PR to my fork won't work either, as the changes are currently being tested on a different branch than master).

ntoll commented 8 years ago

See 261eab6fe1e2827cc44ae16e5a9b4d4dc53a0e54 ;-)

ntoll commented 8 years ago

It appears fixed: https://travis-ci.org/ntoll/mu/builds/103110136 :-)

carlosperate commented 8 years ago

Just one left, remember that for the moment (until I fix the issue with the linux server) it is running on my fork: https://travis-ci.org/carlosperate/mu/jobs/103112763#L360

carlosperate commented 8 years ago

Looks like your commit has also, somehow, fixed the issue the linux server had running py.test at all. Btw, I'd recommend to run the make check locally as well before I PR the travis CI tests, as last time I run it it indicated a few formatting corrections in setup.py.

ntoll commented 8 years ago

Yay! :-)

I say in https://github.com/ntoll/mu/blob/master/CONTRIBUTING.rst to make check before pushing.

carlosperate commented 8 years ago

I say in https://github.com/ntoll/mu/blob/master/CONTRIBUTING.rst to make check before pushing.

Yes, and the final CI script will be running make check instead of the currently test implementation which only runs make test for the moment.

What I meant is that make check fails in the current codebase, indicating minor formatting issues on the setup.py file,

ntoll commented 8 years ago

It passes locally on master for me. I presume you're referring to your own branch..?

carlosperate commented 8 years ago

Last time I runned it today I got the following pep8 issues: https://travis-ci.org/carlosperate/mu/jobs/103096880#L345

ntoll commented 8 years ago

I think that's been fixed by Kushal.

carlosperate commented 8 years ago

Current master has spaces around the equals sign and no space after the commas: https://github.com/ntoll/mu/blob/6499f09447afb6018776f0c8d6e0e368cb2df2b6/setup.py#L26-L28

ntoll commented 8 years ago

Doh... fixed. :-)

carlosperate commented 8 years ago

Awesome, we are almost there, don't forget there is still one assert_called_once lurking around: https://github.com/ntoll/mu/blob/6499f09447afb6018776f0c8d6e0e368cb2df2b6/tests/test_interface.py#L213

Also, for some reason the linux run is throwing the following pyflakes errors, while the OS X one doesn't seem to care: https://travis-ci.org/carlosperate/mu/jobs/103125090#L600-L602

ntoll commented 8 years ago

Fixed the assert_called_once problem.

Looking at pyflakes - it appears to be because you're using get-pip.py to, er, get pip. Once got, if you delete it then the errors should disappear. Viz. package/install_linux.sh

E.g. perhaps line 7 should be something like (off the top of my head):

sudo rm get-pip.py

???

The bottom line is get-pip.py is a third party script that we can't control the PyFlakiness of. ;-)

carlosperate commented 8 years ago

You are completely right, I should have noticed pyflake was complaining about get-pip.py, removing the file should take care of that. We've got the OS X CI server passing all checks, yay! https://travis-ci.org/carlosperate/mu/jobs/103163056 But form some reason the linux one fails again while collecting the files for test: https://travis-ci.org/carlosperate/mu/jobs/103163059#L531-L537 :confused:

ntoll commented 8 years ago

Hmm... that has me stumped. As you can see, the coverage package has caused a core dump. :disappointed:

This isn't good simply because if this were a Python error then we'd get a Python stack trace. Something lower level is amiss.

I suspect this is perhaps related to us using sudo to pip install things..? Perhaps there's a permissions problem?

In any case, I'd suggest non-sudo pip installing and running make check in a virtualenv so we can discount permissions as the cause of the problem.

Does this help..?

(I'm developing this all on Debian Jessie and make check works in a virtualenv).

carlosperate commented 8 years ago

The problem with sudo on this linux server is that pip3 won't install without it, but if I run sudo make check it does still show the same issue (this was already a headache, as sudo environmental variables were different the normal user and it was installing pyqt in a different python directory, which is the main reason we just use the default installation instead of installing 3.5).

You can see on the links posted in PR https://github.com/ntoll/mu/pull/67 (pytest . error, and make check error), that it seems like pytest is collecting the wrong files, and failing when trying to parse those?

At the moment we are not running virtual envs on the CI server mostly because the difficulties I had with installing pyqt correctly using them. Travis CI does a lot of virtualenv configuration on the background to facilitate the process of testing with multiple python versions, so while it might be possible to get it working properly, it will definitely take quite a bit of fiddling around. Do you think a virtual env could really help solve this issue?

ntoll commented 8 years ago

I don't know that much about the machinations of Travis CI.

The fact that the result is a core dump suggests something pretty fundamental going wrong. My first instinct (ergo my suggestion) was to re-create my known-to-be-working environment on Travis (hence the suggestion for a virtualenv).

Other than that, I'm not sure... perhaps just make test and see what happens? (That'll just run the test suite without the PEP8, PyFlakes and coverage support - to be honest, when reviewing PR's we humans should run the full make check as part of the review process.)

Make sense..?

As always, your sterling efforts are most appreciated!

ntoll commented 8 years ago

OK... it's now failing for Linux after I just pushed an update...

https://travis-ci.org/ntoll/mu/jobs/103654484

The command "if [ "$TRAVIS_OS_NAME" = "linux" ]; then bash package/install_linux.sh; fi" failed and exited with 1 during .

???

carlosperate commented 8 years ago

If the task decides to start any time today... hopefully this should have fixed it: https://travis-ci.org/ntoll/mu/jobs/103679087 I had to restart it already once, as it produced an empty log, looks like the travis servers might be under high load.

carlosperate commented 8 years ago

It's been a while since we looked into this. As a quick summary, the make check tests are working in OS X, but we have issues on the Linux Travis server, and we didn't really try to run these on the Windows server.

This week I was playing around with having a build/CI server running on raspberry pi and encountered an issue in the same area, so I created the ci_tests branch/PR to investigate. Haven't had the chance yet to look into it, but I'll post the general info here for now.

The Windows error: https://ci.appveyor.com/project/carlosperate/mu/build/1.0.154#L127 The Linux error: https://travis-ci.org/ntoll/mu/jobs/117199077#L603 The Raspberry Pi error (slightly different):

>>> make check
rm -rf build
rm -rf dist
rm -rf mu.egg-info
rm -rf .coverage
rm -rf docs/_build
find . \( -name '*.py[co]' -o -name dropin.cache \) -print0 | xargs -0 -r rm
find . \( -name '*.bak' -o -name dropin.cache \) -print0 | xargs -0 -r rm
find . \( -name '*.tgz' -o -name dropin.cache \) -print0 | xargs -0 -r rm
find . \( -name _build -o -name var \) -type d -prune -o -name '*.py' -print0 | xargs -0 -r -n 1 pep8 --repeat --exclude=build/*,docs/*,mu/contrib* --ignore=E731,E402
find . \( -name _build -o -name var -o -path ./docs -o -path ./mu/contrib \) -type d -prune -o -name '*.py' -print0 | xargs -0 -r pyflakes
py.test --cov-config .coveragerc --cov-report term-missing --cov=mu tests/
============================= test session starts ==============================
platform linux -- Python 3.4.2, pytest-2.9.0, py-1.4.31, pluggy-0.3.1
rootdir: <http://192.168.5.6:8081/job/mu/ws/,> inifile:
plugins: cov-2.2.1
Makefile:34: recipe for target 'coverage' failed
make: *** [coverage] Aborted
Build step 'Execute shell' marked build as failure
carlosperate commented 6 years ago

The appveyor errors running the tests were fixed in https://github.com/mu-editor/mu/pull/191, and we have https://github.com/mu-editor/mu/issues/244 to investigate the Travis linux issues, so since we had 100% coverage from a while ago, we can close this issue.