Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

TravisCI: ReportLab 3.5.0 breaks tests under Python 2.7 and 3.5 #1737

Closed
peterjc opened this issue Jul 17, 2018 · 12 comments
Closed

TravisCI: ReportLab 3.5.0 breaks tests under Python 2.7 and 3.5 #1737

peterjc opened this issue Jul 17, 2018 · 12 comments

Comments

@peterjc
Copy link
Member

peterjc commented Jul 17, 2018

Noticed on some recent pull requests, e.g. #1735 (comment)

Python 2.7.14,

======================================================================
FAIL: test_partial_diagram (test_GenomeDiagram.DiagramTest)
Construct and draw SVG and PDF for just part of a SeqRecord.
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/travis/build/biopython/biopython/Tests/test_GenomeDiagram.py", line 770, in test_partial_diagram
    == gdd.write_to_string('PDF').replace(b"\r\n", b"\n")
AssertionError
----------------------------------------------------------------------

and Python 3.5,

======================================================================
FAIL: test_partial_diagram (test_GenomeDiagram.DiagramTest)
Construct and draw SVG and PDF for just part of a SeqRecord.
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/travis/build/biopython/biopython/Tests/test_GenomeDiagram.py", line 770, in test_partial_diagram
    == gdd.write_to_string('PDF').replace(b"\r\n", b"\n")
AssertionError
----------------------------------------------------------------------

Given the timing it looks like the release of ReportLab 3.5.0 earlier this week is to blame, see https://pypi.org/project/reportlab/#history

e.g. Our weekly TravisCI cron job failed on the previously passing master branch:

https://travis-ci.org/biopython/biopython/builds/404526057

peterjc added a commit to peterjc/biopython that referenced this issue Jul 17, 2018
This is a tempory workaround, we need to understand
what has changed in ReportLab, and if it is their
bug report it to them. Reference GitHub issue biopython#1737.
@peterjc
Copy link
Member Author

peterjc commented Jul 17, 2018

Testing a workaround by pinning ReportLab 3.4
https://travis-ci.org/peterjc/biopython/builds/404681877

Note we currently only install ReportLab under Python 2.7 and 3.5, so that is why only those failed.

@peterjc
Copy link
Member Author

peterjc commented Jul 17, 2018

Workaround in place - but someone needs to investigate how any why ReportLab 3.5.0 broke our test, and if we need to fix something on our code or not. Help welcome.

brandoninvergo pushed a commit to brandoninvergo/biopython that referenced this issue Jul 17, 2018
This is a tempory workaround, we need to understand
what has changed in ReportLab, and if it is their
bug report it to them. Reference GitHub issue biopython#1737.
@rtf-const
Copy link
Contributor

I finally realised the issue.
The assertion that fails is the one that simple checks that PDF file saved in a file binary equals to PDF saved as string. But reportlab add unique ID to each file, value of ID is depending on current timestamp, so the file generated now and after millisecod will be different because of different PDF ID.
But there is a way to switch out generating ID based on timestamp by setting "invariant" variable in reportlab config to true.
I just created the pull request #1753 that passes the tests without holding down reportlab version

@peterjc
Copy link
Member Author

peterjc commented Aug 11, 2018

Good work @rtf-const :)

@peterjc
Copy link
Member Author

peterjc commented Aug 11, 2018

This seems to affect AppVeyor as well,

======================================================================
FAIL: test_partial_diagram (test_GenomeDiagram.DiagramTest)
Construct and draw SVG and PDF for just part of a SeqRecord.
----------------------------------------------------------------------
Traceback (most recent call last):
  File "C:\projects\biopython\Tests\test_GenomeDiagram.py", line 770, in test_partial_diagram
    == gdd.write_to_string('PDF').replace(b"\r\n", b"\n")
AssertionError
----------------------------------------------------------------------

Seen on a couple of pull requests recently.

@peterjc
Copy link
Member Author

peterjc commented Aug 13, 2018

Pull request #1753 should have fixed the TravisCI problem, let's see what AppVeyor does...

@replabrobin
Copy link

Hi, I am maintainer of the reportlab package and this is a reverse bug pointed out to me by a debian person. It seems the issue here is actually a repeatability issue. The difference between the two versions of GD_region_linear.pdf is caused by an id embedded in the document so in one case it is

/ID [<9737910ac7bddb2a43b171782188193c> <9737910ac7bddb2a43b171782188193c>]

and in the other

/ID [ ]

the ID contains a timestamp. So you need to fix the timestamp using environment variable SOURCE_DATE_EPOCH. I attach a patch to latest code to fix.

test_GenomeDiagram.patch.txt

FWIW I'm not sure why you are messing with '\r\n' etc etc. PDF is a binary format and should always be read/written as bytes. Writing as a text file in windows may well break the document for that reason. Hopefully

@peterjc
Copy link
Member Author

peterjc commented Aug 13, 2018

Thanks Robin - following up on this from the ReportLab side is a welcome surprise.

Your patch sets a specific date, while @rtf-const toggled this setting:

from reportlab import rl_config
rl_config.invariant = True

Our Linux (TravisCI) and Windows (AppVeyor) tests are now passing.

As to the \r\n special case in the comparison, that dates back to cfc38ce and may reflect some confusion at the time when I was getting things to work under Python 2 and 3. Looking at that now, we should probably open the file in binary mode, and compare the byte strings (rather than opening in text mode where Python would by default try to do magic with line endings).

@peterjc peterjc closed this as completed Aug 13, 2018
peterjc added a commit to peterjc/biopython that referenced this issue Aug 13, 2018
@replabrobin
Copy link

Hi Peter, the alternative using rl_config is another way to do this; it changes other things in the PDF as well eg it removes pdf comments that might have python ids in so is probably preferred for this case.

@peterjc
Copy link
Member Author

peterjc commented Aug 13, 2018

Test cleanup applied as c7b909e - thanks @replabrobin

@peterjc
Copy link
Member Author

peterjc commented Nov 6, 2018

Cross reference Debian issue:
https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=913064

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants