← Back to team overview

openlp-core team mailing list archive

Re: [Merge] lp:~ldeks/openlp/ldeks-bugfix into lp:openlp

 

Hi,

I tried creating a unit test for this patch, but I ran into a lot of
problems:

1. song.book is what is throwing the error.  To test this, I need to create
a song without a book attribute and hand it somehow into self.do_import.
song comes from source_songs in line 178 of
openlp/plugins/songs/lib/importers/openlp.py:
    source_songs = self.source_session.query(OldSong).all()

2. self.source_session comes from line 110, which wraps the engine on line
107:
    engine = create_engine(self.import_source) # A sqlalchemy command

3. self.import_source is easy to override, but since it expects a
sqlalchemy database (see line 105), it seems to need a faked-out database
where songs has no book column.  I don't know how to fake this database.
Perhaps using OpenLP to write out a one-song database that we keep in the
repo with the tests?!  Maybe that makes no sense.

I don't think I currently have the right expertise to make this unit test
since I'm new to OpenLP and I'm not really experienced with sqlalchemy. Any
ideas?

Thanks.

Laura

On Sat, Dec 10, 2016 at 2:45 AM, Tim Bentley <tim.bentley@xxxxxxxxx> wrote:

> You will also need a test of some sort.
> All merges need to have them to increase the test coverage.
> They do not need to be vast but a single test will help .
> --
> https://code.launchpad.net/~ldeks/openlp/ldeks-bugfix/+merge/312962
> You are the owner of lp:~ldeks/openlp/ldeks-bugfix.
>

-- 
https://code.launchpad.net/~ldeks/openlp/ldeks-bugfix/+merge/312962
Your team OpenLP Core is subscribed to branch lp:openlp.


Follow ups

References