Remove tests that don't test anything

[rallytime]

These tests are mocked so heavily that they're essentially asserting that the functions return an empty list/dict when we can't connect to the service - no other logic is tested, so I am removing them.

[rallytime]

@tonybaloney Similar situation with these tests here as with the GCE test. Just wanted to ping you here so you're aware that I removed these specific ones, because of the cert mock.

[tonybaloney]

Ok. These were put in for the same issue as the GCE tests. We've had different bugs with both modules. Neither of which are covered in unit or integration. I've started a better way of testing the GCE suite in another PR, it's just a start, it's hard to retroactively add unit tests cos you assume there aren't any bugs.

[tonybaloney]

It would be a lot easier to test and avoid all the monkey patching if the modules were based on classes and DI but I'm going to lose that argument!!
Also we can't assume that the unit tests have any deps installed other than the ones on the requirements folder is that right?
Libcloud actually has a really good unit test setup so I could use the driver that talks to a fixture based static API. But that means it has to be installed first. Is that a fair assumption? @rallytime

[rallytime]

Definitely. We'll get the integration tests running up on Suse. If you need any help with the GCE test PR, let me know. I think the solution that is implemented there will also apply to this file. I'd be happy to help take a look at these tests again once the GCE solution is ironed out, if you like.

[tonybaloney]

It would be really great to have some structure around what to test and where.
So in unit-

• does it check for requirements properly and safely report missing deps
• do any internal utility methods work as designed
• do any connection factories operate from config correctly
  Etc

Integration - now assume that requirements are installed

• does it work with the specified version(s)
• does it handles underlying connection losses gracefully
• does it handle basic functionality

Those are just what I would put in each test part. I've been trying to get unit tests to run (not) on a master by adding the unit test runner a couple of months ago since they were sharing the integration test runner.

This would also really help other module authors to have this in stone. I'm planning to create a module QuickStart script next week that boilerplates different module types based on a question script. I'll raise the PR when it's done but hopefully that could form some sort of best practice for authors in time.

[rallytime]

@tonybaloney We actually have lots of unit tests that rely on external libs. Such as mysql unit tests - those need the mysql python bindings and such installed first. The only catch there is just that we gate the deps with a skipIf at the top of the test case class.

So something like this is totally doable:

try:
    import libcloud
    HAS_LIBCLOUD = True
except ImportError:
    HAS_LIBCLOUD = False


@skipIf(HAS_LIBCLOUD is False, 'Libcloud is required to run these tests')
class MyTestClass....

Which we can then do the same thing for whatever unit tests libs you need for libcloud. The boto unittests are a great example of this actually, which depend both on the boto library to be installed, along with moto, which is the unittest mock lib for boto.

Once those deps are added, we'll need to make some changes to our jenkins test VM states to make sure the tests run on our autotest setup, but that's easy enough.

[tonybaloney]

Ah sweet! That's easy then. I can test them properly!

[rallytime]

@tonybaloney There are a couple of sections on when to write unit and when to write integration tests and how those play together for test coverage - But they can always use improvements! I'll create a new issue with your latest comments to include those. Thank you for the suggestions.

For reference, the docs I am referring to are here:

• Writing Unit Tests
• Integration Tests
• Salt Test Suite Tutorial

[rallytime]

Oh, also, you should be able to run unit tests without a master installed already. There's some quirkiness with how you run them, but basically you need to be in the tests/ dir and run them from there with either -u (or --unit) or you can run individual files/classes/tests with -n. As long as you do python runtests.py -n unit.cloud.clouds.dimensiondata_test instead of python tests/runtests.py (that will kick off the test daemons, which do require the master).

Anyway, any improvements in testing is always welcome and awesome. I know that isn't quite what you meant in your quickstart script idea, but I just wanted to let you know that was possible.

[tonybaloney]

#32792 This is the lightweight unit test runner. I'm the only one using it so far!
The integration runner has tons of import and setup that shouldn't be required for unit tests