Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Unit testing framework and example #192

Merged
merged 2 commits into from
Jul 14, 2018

Conversation

stephendade
Copy link
Contributor

There's two commits here:

  1. Tweaks to the unit testing framework such that all tests in the ./tests folder and following the test_*.py pattern are executed by Travis.
  2. Unit tests for rotmat.py, with a few patches to support testing. Basically an example of the sorts of tests I'll be writing.

A few things from the rotmat tests:
-I've found some functions (like from_euler312() and to_euler312()) don't actually produce the correct output (unless I messed up the test). I've not seen these functions used elsewhere in pymavlink, so the incorrectness hasn't been noticed.
Do we remove unused and incorrect functions?

-Some functions, such as Matrix3.rotate() give very little on expected inputs/outputs (and the associated maths background), making then difficult to verify for correctness. I could, of course just assume that it produces correct output now and use it as a baseline. This function appears to be only used for some DCM simulations in mavextra.py - not sure if anyone is still using this.

I suppose I'm looking for some agreed guidance on whether I should:
a) Assume all back-end functions are currently correct and build tests based on that
b) Start removing any back-end functions that produce incorrect output AND are unused in pymavlink?
c) Something in-between. Maybe a case-by-case basis via pull requests?
(I'd prefer, where possible to avoid spending my efforts fixing code that nobody's actually using)

@stephendade stephendade mentioned this pull request Jun 21, 2018
@peterbarker
Copy link
Contributor

peterbarker commented Jun 28, 2018 via email

@stephendade
Copy link
Contributor Author

stephendade commented Jul 4, 2018

Probably not - some people may be using them? It looks like these are
duplicated in pysim/rotmat.py - and those ones are used. Perhaps you
could compare with those?

Ahh, the tests in that file work quite nicely. I'll use those.

Check out ArduPilot's autotest - they may be used in there.

Couldn't find them in there (or anywhere else in Ardupilot). I've spent some time going over the function and managed to write a unit test for it, so all good now.

I've fixed up my commit, so this should be good to be merged now :)

@peterbarker
Copy link
Contributor

The unit tests currently fail?

The assertions also don't look quite right. Using assertTrue(a==b) ubiquitously means a lot of context is lost. I believe you should be able to assert a == b and get more meaningful test failure messages.

@stephendade
Copy link
Contributor Author

I believe you should be able to assert a == b and get more meaningful test failure messages.

Yep, I've now fixed that.

The unit tests currently fail?

They're running fine on my laptop and Travis is reporting ok...

@peterbarker
Copy link
Contributor

It works for me - but only after I actually install pymavlink. Is that expected behaviour - that it tests the installed pymavlink rather than the pymavlink repository you're running pytest in?

pbarker@bluebottle:~/rc/pymavlink(unittesting)$ pytest 
============================= test session starts ==============================
platform linux2 -- Python 2.7.14, pytest-3.6.3, py-1.5.4, pluggy-0.6.0
rootdir: /home/pbarker/rc/pymavlink, inifile: pytest.ini
plugins: mock-1.10.0
collected 24 items                                                             

tests/test_quaternion.py ..............                                  [ 58%]
tests/test_rotmat.py ..........                                          [100%]

========================== 24 passed in 24.98 seconds ==========================
pbarker@bluebottle:~/rc/pymavlink(unittesting)$ 

@stephendade
Copy link
Contributor Author

stephendade commented Jul 14, 2018

It works for me - but only after I actually install pymavlink. Is that expected behaviour - that it tests the installed pymavlink rather than the pymavlink repository you're running pytest in?

Yes, it's the expected behaviour. Pymavlink needs to be installed first to get the imports correct - it is always importing from the package rather than the source. The test files themselves are not affected my this (as they're not installed files)

I'd assumed that it's "the way" pymavlink is architectured. There's alot of from pymavlink import xxx in the source (which only works when pymavlink is installed)...

EDIT:
To give an example, DFReader.py in has the line from . import mavutil. Running DFReader without pymavlink installed will generate an import error.

@peterbarker peterbarker merged commit eea8e11 into ArduPilot:master Jul 14, 2018
@peterbarker
Copy link
Contributor

@stephendade The example you give is one of the no-nos in modern Python, I think. It's certainly not something I like...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants