Skip to content

Conversation

@Alexey-ebi
Copy link
Contributor

First part of unit and integration tests

@Alexey-ebi Alexey-ebi requested a review from wizardfan May 8, 2019 09:11


class TestUnitls(unittest.TestCase):
def test_create_logging_instance(self):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

to me, this test is not sufficient. The create logging function is to write logs, which has not been tested.
Proposed:

  1. write something to the file
  2. read the file (filename could be worked out)
  3. check the content
    Not sure whether it is the right version of having to_file option though

}
},
]
self.assertEqual(utils.get_number_of_published_papers(data)['yes'], 2)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

personal choice:
assertDictEqual which indicates clearly that a dict is expected
expected = {
'yes': 2,
'no': 2
}
self.assertDictEqual(utils.get_number_of_published_papers(data),expected)
applies to all other methods

logger.error(f"Error when try to insert into index {es_index_prefix}{doc_type}: " + str(e.args))
pprint.pprint(body)


Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the commit message is not suitable for the changes
Actually could be merged to the next commit

@wizardfan
Copy link
Contributor

General suggestion:

  1. Insufficient sample data: normally only provide 1 sample, better to have several samples. For example, test for specimen summary, 5 samples (2 females, 3 males), (1 cattle, 3 goats, 1pig) etc..

  2. save test data into files and if possible, order the field keys (not just in random order as from ES). it is ok to have inline data if only one sample record.

  3. As many tests are similar, sample input, expected result, one line method call and compare the results. Would it be possible to have a general test method which has a list of methods to test and provides file names for the input and expected result files?

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.

3 participants