Code review comment for lp:~gmb/maas-test/report-machine-info

Point [2] needs addressing, but it doesn't need re-review. Nice.

[1]

+    def dump_data(self, dump_dir=None):

Please be adding a docstring.

[2]

+    def dump_data(self, dump_dir=None):
+        if dump_dir is None:
+            dump_dir = utils.DEFAULT_LOG_DIR
+        return_code, stdout, _ = self.kvm_fixture.run_command([
+            'sudo', 'maas', 'dumpdata', 'metadataserver.NodeCommissionResult'
+            ])

dump_dir isn't being used for anything. Can it be dropped?

[3]

MAASFixture looks like it's gone all 2008-financial-crisis: too big to
fail, too big to test. It would be good to have a test that confirms
that the data is dumped during clean-up, but I suspect that the noise of
set-up (mocking, etc.) would smother the useful kernel of the test.