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

Implement unit tests #16

Open
3 of 5 tasks
tomchadwin opened this issue Jan 15, 2018 · 15 comments
Open
3 of 5 tasks

Implement unit tests #16

tomchadwin opened this issue Jan 15, 2018 · 15 comments

Comments

@tomchadwin
Copy link
Collaborator

tomchadwin commented Jan 15, 2018

@mtravis So the way this works is that you should write tests in Python, give them a filename starting test_, and put them in test. There are already some there which are provided out of the box by the Plugin Builder. You can then either run those tests individually to check they pass, or use nose to run them all at once.

@akbargumbira (I think) wrote a post describing how to run such tests on Windows, but generally, they only run on Linux/Mac.

Here's an initial list of possible tests, to expand as we think of more:

  • xy_to_osgb gives a correct result
  • xy_to_osgb outside BNG coverage behaves gracefully
  • xy_to_osgb with different precision gives results with the correct precision
  • Changing precision spinner changes precision setting
  • Clicking with the tool writes to the clipboard
@tomchadwin
Copy link
Collaborator Author

I'm not sure what we want to test with this, as I've never used it. Who can think of some test cases?

@mtravis
Copy link
Owner

mtravis commented Jan 15, 2018

@sklencar any ideas?

@mtravis
Copy link
Owner

mtravis commented Jan 15, 2018

Essentially the plugin uses the clicked map co-ords and converts them using the xy_to_osgb function.
Could we feed a set of co-ords into that to test to see it is outputting correctly?

@tomchadwin
Copy link
Collaborator Author

tomchadwin commented Jan 15, 2018

Yes. The idea of unit testing is to break tests down into multiple smaller tests, each of which only tests one thing. So, off the top of my head:

  1. Clicking a coordinate gets the right result
  2. Clicking outside BNG coverage behaves gracefully
  3. Clicking with different precision gives results with the correct precision

Have you had many bugs or problems? If you have, think of tests which would have spotted them. If you can cover most of the code with tests, it will make it easier to handle eg QGIS API changes.

The bad news is that my experience of writing QGIS tests is very specific - changing various settings, and ensuring qgis2web exports given code. In other words, my tests always end with comparing an output string to a control. Some of your tests will do that, but others might do something else. We'll see what we come up with.

@sklencar
Copy link

Tests above look reasonable. I would like to add:
4. "if value has changed in precision spin field, it has changed also the tool precision". Its a related to 3. test, but I guess that one should cover only the convert function.
5. Clicking a coordinate override clipboard with current converted value

@tomchadwin
Copy link
Collaborator Author

Is 5 the current behaviour? I think that should be configurable by checkbox, and default to off - people could lose data.

@sklencar
Copy link

Yes, so far it works like that. After opening bbox with info about point a user clicked on, the clipboard is always overwritten. Agree, that it should be configurable.

tomchadwin added a commit that referenced this issue Jan 15, 2018
@tomchadwin
Copy link
Collaborator Author

tomchadwin commented Jan 15, 2018

OK, first test is there, runs on Travis, and passes. The test itself is the function test_xy_to_osgb:

https://github.com/mtravis/gridref-qgis/blob/master/test/test_gridref.py#L32

To write further tests, add more functions after it, within QGISTest, and use the different unittest assert methods to test for the desired results.

Hope this is comprehensible. I'll close this issue, but do ask question here if not.

@tomchadwin
Copy link
Collaborator Author

Right, I'll perhaps try to add one of those markdown checkbox lists to the first post here of possible tests, so we can keep track of what we've done, and add others if we think of them.

@mtravis
Copy link
Owner

mtravis commented Jan 16, 2018

We could run a test to check out the "point out bounds" works.

I think also that they want to add this tool to processing toolbox so we should have tests for batch processing but @sklencar will know more about that.

tomchadwin added a commit that referenced this issue Jan 16, 2018
tomchadwin added a commit that referenced this issue Jan 16, 2018
@sklencar
Copy link

@mtravis Yes, I got requirement to move the tool to processing toolbox and get rid of the button in the action toolbar.

@tomchadwin
Copy link
Collaborator Author

Second test implemented and passes.

tomchadwin added a commit that referenced this issue Jan 16, 2018
tomchadwin added a commit that referenced this issue Jan 16, 2018
@tomchadwin
Copy link
Collaborator Author

Third test implemented and passes.

tomchadwin added a commit that referenced this issue Jan 16, 2018
@tomchadwin
Copy link
Collaborator Author

Potential issue with proposed test 4, "Changing precision spinner changes precision setting":

I don't think "precision" is stored anywhere as a var independent of the spinner - functions read the spinner state directly. That means that you'd only be testing that chaging the spinner changes the spinner, which seems unnecessary.

I guess if the plugin is rewritten as a processing algorithm as discussed, and the GUI is therefore decoupled from the functions, then this test will become meaningful.

@tomchadwin tomchadwin reopened this Jan 16, 2018
@tomchadwin
Copy link
Collaborator Author

Proposed test 5, "Clicking with the tool writes to the clipboard":

This is a more complex test, as it will require using QTest to click() a known point within OSGB coverage. This in turn means setting the canvas up correctly, including extent, zoom, and CRS, and clicking the correct screen pixel coordinates.

tomchadwin added a commit that referenced this issue Jan 16, 2018
#16 - nose doesn't handle a line break, evidently
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

No branches or pull requests

3 participants