voxpupuli / puppet-collectd

Collectd module for Puppet
https://forge.puppet.com/puppet/collectd
Apache License 2.0
69 stars 273 forks source link

mongodb plugin tests are possibly brittle #373

Open ffrank opened 8 years ago

ffrank commented 8 years ago

There was a short discussion of regex tests vs. fixtures in #371.

I still quite strongly disagree with @ghoneycutt's assertion that fixtures fix a problem with the data structures at hand.

If I understand you correctly, you fear that regex tests are too broad and don't take the structure of the template output into account. That is, a change to the template that would case invalid output due to broken XML structure would pass the tests.

I argue that this distinction is not the point of unit tests. If we want to make sure that certain inputs lead to output that is usable by collectd, we'd need to add some actual acceptance tests. It is out of the scope of unit testing IMO.

I propose to replace the checks in the current set of unit tests to make weaker assertions. I would also add an integration test to check the XML structure.

All that being said, thanks for being so thorough about testing, it really adds much value to your code contribution!

jyaworski commented 8 years ago

Can we get more information, here? The latest modulesync added some foundation for acceptance testing.

ffrank commented 8 years ago

I will eventually go ahead and change the testing approach. This is a low priority, however.

juniorsysadmin commented 7 years ago

If we want to make sure that certain inputs lead to output that is usable by collectd, we'd need to add some actual acceptance tests. It is out of the scope of unit testing IMO.

I agree