search

jseutter / ofxparse

Ofx file format parser for Python
http://sites.google.com/site/ofxparse/
MIT License
204 stars 121 forks source link

Flawed tests in test_parse.py? #95

Closed rdsteed closed 7 years ago

rdsteed commented 8 years ago

I've written an extension to Beautiful Soup 4 (bs4) which I believe is much more robust at handling ofx formatted files than any of the various options currently available in Beautiful Soup 4. When I test it in ofxparse using nosetests, it passes all tests except testThatParseStmtrsReturnsAnAccount and testThatReturnedAccountAlsoHasAStatement.

Both contain the line:

account = OfxParser.parseStmtrs(stmtrs.find('stmtrs'), AccountType.Bank)[0]

In looking at the definition of parseStmtrs, it appears the first argument (stmtrs_list) should be a list of stmtrs Tags rather than the single Tag returned by stmtrs.find('stmtrs'). When I change the line to:

account = OfxParser.parseStmtrs(stmtrs.find_all('stmtrs'), AccountType.Bank)[0],

the tests work fine. I believe this is because because stmtrs.find_all('stmtrs') returns a list of stmtrs Tags as expected by parseStmtrs.

Is this a case of a malformed test lurking in the repository, or is it simply my thoughts that are malformed?

jseutter commented 8 years ago

It's a badly formed test. The relevant code in OfxParser.parse() is

   stmtrs_ofx = ofx.findAll('stmtrs')
   if stmtrs_ofx:
       ofx_obj.accounts += cls_.parseStmtrs(stmtrs_ofx, AccountType.Bank)

...which passes in a list of stmtrs.

I can fix the test, or you can submit a fix if you'd be so inclined :)

Thanks for pointing this out!

rdsteed commented 8 years ago

Fix incorporated as part of Pull Request #96