fedora-static-analysis
add ORM for Firehose objects
Hi @davidmalcolm, we discussed with @paultag about integrating the orm used in firewoes directly into firehose. Tell me what you think about this idea, and if you want things to be changed :-)
Thanks. Is there any automated test coverage for this? i.e. can we have, say, a tests/test_orm.py that exercises things against an in-memory sqlite db? (does this already exist within the firewoes code?)
Yes, we have this in firewoes. But I need a data set, do you think the xml files in examples/ are enough or should I add those we use?
Looking at firewoes, presumably you mean the xml files in: https://github.com/Debian/firewoes/tree/master/tests/data , right? I think I'd like some test cases that use the xml files in the firehose "examples" directory, they are formatted for human-readability with indentation and comments, and I know that they exhibit various features - some have optional elements, others omit optional elements etc, which is good for testing. The firewoes ones may also have these properties, but I can't easily see that since the files are each all on a single line.
That said, the firewoes examples look like real examples, and there's merit to that, and any testing is better than no testing :)
Also, is the firewoes test suite: https://github.com/Debian/firewoes/blob/master/tests/tests.py ? That seems to exercise both the model and the controller layers (with firewoes code); for firehose itself I don't want to add a dependency on firewoes, obviously. I'd prefer a test suite that merely exercises the model layer: create a sqlite db, populate it with examples, and query it, going from XML to objects to rows and then back again to objects - if that makes sense.
Perhaps we could also have tests that simply create some objects in a loop, using logic to establish properties e.g. 10 issues, of which 3 are in one file, 7 in another, and then run a query and assert that we get the right thing back?
Yes I meant these xml files in firewoes/test/data, which were more intended for firewoes anyway, those in examples/ make indeed more sense. Yes the firewoes test suite is in firewoes/tests/tests/py, and the model/controller layers are used because I test the web app in its integrity there, especially firewoes/lib/hash.py, used to distinguish different Firehose objects, and we don't need this in Firehose. So, using the examples/* + generated issues, all of this in a sqlite in-memory db, seems ok for me :-) I'll do this! Thanks for the feedback!
Should I add 'sqlalchemy' in requirements.txt and make travis.yml install them?
Since I use single table inheritance for firehose.model.{SourceRpm, DebianBinary, DebianSource}, the attribute "buildarch" exists for the three classes when they are mapped, making https://github.com/fedora-static-analysis/firehose/blob/master/tests/test_model.py#L550 fail. I can think about two solutions:
- separate the runs of tests/test_model.py and tests/test_orm.py, instead of using unittest discovering (which loads firehose.orm),
- delete this assertion in test_debian_source.
What do you think?