Issue1036 - Add unit tests for AM code #1044
Conversation
…ork. Other use cases are commented for now
…n names and longer bulletin headers
Test Results220 tests 212 ✅ 17s ⏱️ Results for commit 57f1f57. |
|
This looks great! Lots of good testing here for individual cases you've seen; not something I've been able to do thus far. Only observations I'd make are... 2 - You're building an |
| class Options: | ||
| def __init__(self): | ||
| # self.o = sarracenia.config.default_config() | ||
| self.logLevel = "DEBUG" | ||
| self.logFormat = "" | ||
| self.queueName = "TEST_QUEUE_NAME" | ||
| self.component = "flow" | ||
| self.config = "foobar_am.conf" | ||
| self.sendTo = "am://127.0.0.1:5005" | ||
| self.pid_filename = "/tmp/sarracenia/am_test/pid_filename" | ||
| self.directory = "/tmp/test/directory" | ||
| self.housekeeping = float(39) | ||
| self.fileAgeMin = 0 | ||
| self.fileAgeMax = 0 | ||
| self.post_baseUrl = "http://localhost/" | ||
| self.post_format = "v02" | ||
|
|
||
| def add_option(self, option, type, default = None): | ||
| if not hasattr(self, option): | ||
| setattr(self, option, default) | ||
| pass |
There was a problem hiding this comment.
This should probably be replaced by importing sarracenia.config, and then instantiating that on a per-test bassis, setting the options needed.
If you're going to be re-using the same set of options universally, then you can create a fixture, or method for it.
| from base64 import b64encode | ||
|
|
||
| BaseOptions = Options() | ||
| renamer = Raw2bulletin(BaseOptions) |
There was a problem hiding this comment.
Since you're calling this external method, it might be good to add a dependency for it above these tests, but that would mean writing unit tests for that other method.
Alternatively, re-scope your tests here to only cover what gather/am is doing.
There was a problem hiding this comment.
Hm.. I see Raw2Bulletin, and Bulletin are getting used in __init__ as well, so maybe we ignore these dependencies until tests are written for those modules.
There was a problem hiding this comment.
Yeah. Unfortunately, we need both Bulletin and Raw2Bulletin for a lot of the tests. The scope of these unit tests are mostly for the "bulletin processing and renaming" functionality
But you're right we'll need unit tests for these classes as well. I'll make sure to take note of that.
There was a problem hiding this comment.
you just end up with unit tests for Bulletin and Raw2Bulletin... still a good thing.
I mean most of the problems you ran into were bulletins with unexpected content, so those tests are doing useful stuff. It's at worst slightly mislabelled.
| from base64 import b64encode | ||
|
|
||
| BaseOptions = Options() | ||
| renamer = Raw2bulletin(BaseOptions) |
There was a problem hiding this comment.
you just end up with unit tests for Bulletin and Raw2Bulletin... still a good thing.
I mean most of the problems you ran into were bulletins with unexpected content, so those tests are doing useful stuff. It's at worst slightly mislabelled.
|
I think it'd be better to merge now, so at least we'll have something for future PRs |
These unit tests are mostly based off of the errors that were found under #980, adding also testing for regular bulletins and errors from personal observations noted when monitoring logs.
Suggestions are welcome for adding more tests to the repertoire.