Asciidoctor-pdf: test suite failures when building RPM

[snecklifter]

Hello, sorry for ruby newbie question but I see a few failures when running rspec and comparing results with the expected floats.

Obviously this must be something related to local setup but just wondered any pointers?

  4) Asciidoctor::PDF::Converter - Cover Page should not allow page size of PDF cover page to affect page size of document
     Failure/Error: (expect pdf.pages[0][:size]).to eql PDF::Core::PageGeometry::SIZES['LETTER']
       expected: [612.0, 792.0]
            got: [612, 792]
       (compared using eql?)
     # ./spec/cover_page_spec.rb:181:in `block (2 levels) in <top (required)>'
  5) Asciidoctor::PDF::Converter - Font font sizes should resolve font size of inline element specified in rem
     Failure/Error: (expect linked_text[:font_size]).to eql 9.0
       expected: 9.0
            got: 9
       (compared using eql?)
     # ./spec/font_spec.rb:224:in `block (3 levels) in <top (required)>'
  6) Asciidoctor::PDF::Converter - Image SVG should replace unrecognized font family with base font family
     Failure/Error: (expect text[0][:font_size]).to eql 12.0
       expected: 12.0
            got: 12
       (compared using eql?)
     # ./spec/image_spec.rb:187:in `block (3 levels) in <top (required)>'
  7) Asciidoctor::PDF::Converter - Image SVG should replace unrecognized font family in SVG with SVG fallback font family if specified in theme
     Failure/Error: (expect text[0][:font_size]).to eql 12.0
       expected: 12.0
            got: 12
       (compared using eql?)

[mojavelinux]

The test suite runs successfully on both Linux and Windows using CRuby 2.3 - 2.7 and JRuby. Which version of Ruby are you using and on which platform?

[snecklifter]

Thanks Dan, this is Fedora.

✔ /tmp/rubygem-asciidoctor-pdf [master|…1] 
09:39 $ rspec --version
RSpec 3.8
  - rspec-core 3.8.2
  - rspec-expectations 3.8.4
  - rspec-mocks 3.8.1
  - rspec-support 3.8.2
✔ /tmp/rubygem-asciidoctor-pdf [master|…1] 
09:39 $ ruby --version
ruby 2.6.5p114 (2019-10-01 revision 67812) [x86_64-linux]

[mojavelinux]

Then I'd say there is something wrong with the rspec installation, or something in one of the gems. There's no reason that should be happening. It's not Ruby because I can use the system Ruby from Fedora and the tests pass. That's why I'm lead to believe it's rspec. Or maybe it's a problem in another dependency.

[mojavelinux]

Maybe the issue is with pdf-reader.

[mojavelinux]

It should not be turning floats into integers. That shows the test is asserting the right thing.

[mojavelinux]

Are you packaging asciidoctor-pdf? I'm trying to understand how you are running it.

[snecklifter]

Are you packaging asciidoctor-pdf? I'm trying to understand how you are running it.

Yes, we have to exclude a small number of tests which fail like those which require network connectivity which isn't possible from the build system. However I've always been intrigued as to why these tests fail.

https://src.fedoraproject.org/rpms/rubygem-asciidoctor-pdf/blob/master/f/rubygem-asciidoctor-pdf.spec

[mojavelinux]

I would say this is an indication of a broader problem with the Ruby setup on Fedora, then. You shouldn't be getting different results from what you get when using gems installed using bundler.

[mojavelinux]

we have to exclude a small number of tests which fail like those which require network connectivity which isn't possible from the build system

I recommend raising an issue for that to add a NO_NETWORK flag so those tests get excluded without a patch. (Though it should be less now that the test suite has it's own web server).

[mojavelinux]

Did debugging reveal anything?

[snecklifter]

Not had the chance to debug much further. Fedora 32 is going through change freeze and now the failures are up to 16:

https://kojipkgs.fedoraproject.org//work/tasks/607/41420607/build.log

[mojavelinux]

Can you give me the instructions to reproduce on my machine or in a Docker image?

[mojavelinux]

Frankly, the fact that these tests fail on Fedora should be really worrying to the ruby-sig. We're running these tests on a dozen Ruby versions across Windows and Linux and they all pass.

[mojavelinux]

I discovered the issue. Fedora uses a new version of the pdf-core gem (0.8.1) than what prawn requests (0.7.0). There was a change introduced in that version to truncate any float value in the PDF to an integer if the decimal is zero. See https://github.com/prawnpdf/pdf-core/commit/3bea761521b3483e1e81968c600b6fddf6a87863. That's why we're seeing differences in the test results when comparing the page dimensions.

[mojavelinux]

@snecklifter Can you test the PR? https://github.com/asciidoctor/asciidoctor-pdf/pull/1679

[mojavelinux]

I'm marking this as a bug since it's a future bug ;)

[mojavelinux]

I now have master working with pdf-core 0.8.1. I'll port those updates into the PR for the 1.5.x branch tomorrow.

[mojavelinux]

@snecklifter If you change the following line to if false do the CLI tests work?

https://github.com/asciidoctor/asciidoctor-pdf/blob/v1.5.x/spec/spec_helper.rb#L324

[snecklifter]

@snecklifter If you change the following line to if false do the CLI tests work?

https://github.com/asciidoctor/asciidoctor-pdf/blob/v1.5.x/spec/spec_helper.rb#L324

Unfortunately not.

https://kojipkgs.fedoraproject.org//work/tasks/6853/44156853/build.log

[mojavelinux]

That's okay, that tells us something.

We had the same problem with the main Asciidoctor gem. The packaging environment does not properly set up RubyGems, so we cannot rely on those APIs to return meaningful values. Instead, we have to feed it with locations via environment variables. In this case, we need to pass in the bin path.

[mojavelinux]

I noticed the errors with OpenURI.open_uri when I ran the tests on my machine using system Ruby. The tests are assuming that the open-uri-cache gem is available, and when it's not, it causes the OpenURI.open_uri method to be deleted. This is now fixed in the PR.

[mojavelinux]

Please try with the updated patch. We should only get errors for the CLI now.

And on that front, is there an environment variable available that says where the gem is installed, or that we can use to build that path? In other words, how can we get the location of the bin script?

What's strange is that this fails:

Gem.bin_path 'asciidoctor-pdf', 'asciidoctor-pdf'

I don't see why that wouldn't work. But, if necessary, we can build the path another way.

[voxik]

I don't see why that wouldn't work. But, if necessary, we can build the path another way.

I have not digged deep into this, but if the current method works, it can work just for two reasons:

1. The test suite is executed using Bundler, which can fake a lot of stuff and pretend the current repository is gem. So if the test suite is executed without Bundler, this cant work IMHO.
2. The other case, when this could work, is when other version of asciidoctor-pdf is installed on the system.

[voxik]

From the Fedora discussion, I don't understand your assertion:

The build installs the gem before running the tests, so I don't see why this would fail.

If you refer to RPM build, then there is called gem install asciidoctor-pdf, but the gem is not installed into some system wide location, it is installed just within RPM build environment.

[voxik]

If you check the build.log past the Executing(%build), you can see where the installation goes.

[voxik]

But the "real" installation happens after the Executing(%install). That moves the content of the package into the final place, where RPM can pick it up, but it is yet again private RPM build location, which just mimics the real filesystem layout of the target filesystem.

[mojavelinux]

@voxik Aha! Thanks for that clarification. That helps me understand why it wouldn't find the bin script.

Is it possible to access this location via an environment variable (or any other sort of environment or Ruby property)?

/builddir/build/BUILD/asciidoctor-pdf-1.5.0.beta.6/usr/bin

If I can detect that location somewhere, I can use it in the test suite to find the bin script.

[mojavelinux]

The test suite is executed using Bundler, which can fake a lot of stuff and pretend the current repository is gem. So if the test suite is executed without Bundler, this cant work IMHO.

FYI, the logic in the test suite breaks out of Bundler even if it was run via bundle exec when executing the CLI tests. As long as we can resolve the path to the bin script, I'm confident the tests will pass.

(and so far all the other tests are passing, so the way in which the suite is executed isn't the crux of the problem, as far as I can tell).

[voxik]

There is no reason why the test suite could not be executed as PATH=some/path/to/bindir:$PATH rspec spec

[voxik]

In theory, there could also be used GEM_HOME or GEM_PATH that they should consider some other location for gems.

[voxik]

There are also some RPM env variables, but I don't think it would be smart to use them in upstream code (sorry, these are for rubygems package obviously, so just for ilustration :innocent:):

~
RPM_ARCH=x86_64
RPM_BUILD_DIR=/builddir/build/BUILD
RPM_BUILD_NCPUS=4
RPM_BUILD_ROOT=/builddir/build/BUILDROOT/rubygems-2.6.13-105.fc33.x86_64
RPM_DOC_DIR=/usr/share/doc
RPM_LD_FLAGS=-Wl,-z,relro -Wl,--as-needed -Wl,-z,now -specs=/usr/lib/rpm/redhat/redhat-hardened-ld
RPM_OPT_FLAGS=-O2 -g -pipe -Wall -Werror=format-security -Wp,-D_FORTIFY_SOURCE=2 -Wp,-D_GLIBCXX_ASSERTIONS -fexceptions -fstack-protector-strong -grecord-gcc-switches -specs=/usr/lib/rpm/redhat/redhat-hardened-cc1 -specs=/usr/lib/rpm/redhat/redhat-annobin-cc1 -m64 -mtune=generic -fasynchronous-unwind-tables -fstack-clash-protection -fcf-protection
RPM_OS=linux
RPM_PACKAGE_NAME=rubygems
RPM_PACKAGE_RELEASE=105.fc33
RPM_PACKAGE_VERSION=2.6.13
RPM_SOURCE_DIR=/builddir/build/SOURCES
~

[mojavelinux]

Thanks again for the insight.

There is no reason why the test suite could not be executed as PATH=some/path/to/bindir:$PATH rspec spec

Doing that would violate the assertion the test is trying to make, which is that the bin script belongs to the gem (not something that's global to the path).

I don't think it would be smart to use them in upstream code

I think it would be very smart because what we're trying to do here is introduce an exception for running the test suite in a non-standard Ruby environment. I need to know that's what's going on so that I can introduce the exception.

I'll play around and see what I can come up with.

[mojavelinux]

@snecklifter Can you try again with the patch in this PR: https://github.com/asciidoctor/asciidoctor-pdf/pull/1679 ?

That should either get the CLI tests passing or it should get us close enough that we'll know what to do next.

[voxik]

By putting this quirk into asciidoctor-pdf, you take a freedom from Fedora to do what it needs to to. This should be way to go for us:

~
GEM_HOME=/builddir/build/BUILD/asciidoctor-pdf-1.5.3/usr/share/gems rspec spec/
~

This fixes the issues without the patch.

[snecklifter]

@snecklifter Can you try again with the patch in this PR: #1679 ?

Sure, its come back with some name errors:

https://kojipkgs.fedoraproject.org//work/tasks/8019/44188019/build.log

5) asciidoctor-pdf Examples should convert the basic example Failure/Error: File.join rpm_build_dir, ([gem_name, Asciidoctor::PDF::Version].join '-'), 'usr', 'bin', bin_name NameError: uninitialized constant Asciidoctor::PDF::Version Did you mean? Asciidoctor::PDF::VERSION Asciidoctor::VERSION # ./spec/spec_helper.rb:302:in `bin_script' # ./spec/spec_helper.rb:314:in `asciidoctor_pdf_bin' # ./spec/cli_spec.rb:42:in `block (3 levels) in <top (required)>'

[snecklifter]

By putting this quirk into asciidoctor-pdf, you take a freedom from Fedora to do what it needs to to. This should be way to go for us:

GEM_HOME=/builddir/build/BUILD/asciidoctor-pdf-1.5.3/usr/share/gems rspec spec/

This fixes the issues without the patch.

Yes, have tested locally and looks good, just building in koji now.

https://koji.fedoraproject.org/koji/taskinfo?taskID=44189198

The discussions about how best to do this are above my understanding so please let me know what to implement and I shall. :)

[mojavelinux]

GEM_HOME=/builddir/build/BUILD/asciidoctor-pdf-1.5.3/usr/share/gems rspec spec/

I could work with this.

[mojavelinux]

Sure, its come back with some name errors:

I mistyped the constant name.

[mojavelinux]

@snecklifter Can you try Vit's suggestion with the branch as it is now (I rolled back the bin_script resolver change). Let's see if setting GEM_HOME gets us there. Then we'll decide what to do.

[mojavelinux]

+ rspec spec

Please don't specify the directory. The project is configured to use rspec without any arguments.

[mojavelinux]

rspec ./spec/table_spec.rb:380 # Asciidoctor::PDF::Converter - Table Dimensions should not break words in head row when autowidth option is set

This test assumes you are running the test suite against prawn-table from its master. I need to detect this and have the test assert accordingly. Action item for me.

[mojavelinux]

rspec ./spec/image_spec.rb:688 # Asciidoctor::PDF::Converter - Image Remote should read remote image over HTTPS if allow-uri-read is set

I think you need to disable this one too because it requires network access. We could add like a NO_REMOTE environment variable so all remote tests are automatically disabled without a patch.

[mojavelinux]

rspec ./spec/cli_spec.rb:23 # asciidoctor-pdf Require should load converter if backend is pdf and require is asciidoctor-pdf
rspec ./spec/cli_spec.rb:31 # asciidoctor-pdf Require should load converter if backend is pdf and require is asciidoctor/pdf

These are going to fail because they require requiring the gem we are testing, which does require Bundler. I can have them automatically disable if the test suite was not run with Bundler.

[mojavelinux]

@snecklifter Can you add the NO_REMOTE environment variable to the run? I'll update the test suite to disable those tests. Then you don't need to mark them as pending. (If you prefer another environment variable name, let me know, like NO_NETWORK, or whatever).

[mojavelinux]

I fixed the CLI and table tests. Now we're just down the NO_NETWORK / NO_REMOTE situation.

[mojavelinux]

Btw, this test suite verifies that, on Fedora, prawn 2.2.2 works with pdf-core 0.8.1. I don't know why that combination doesn't work when using the RubyGems directly from source...but that's just what's happening. This test suite gives us peace of mind that this combination of RPM gems is functional on Fedora.

[snecklifter]

@snecklifter Can you try Vit's suggestion with the branch as it is now (I rolled back the bin_script resolver change). Let's see if setting GEM_HOME gets us there. Then we'll decide what to do.

Looking good:

https://kojipkgs.fedoraproject.org//work/tasks/5663/44195663/build.log

With regards to the env variable for remote tests, am I setting:

NO_REMOTE=true

or something else?

[snecklifter]

rspec ./spec/image_spec.rb:688 # Asciidoctor::PDF::Converter - Image Remote should read remote image over HTTPS if allow-uri-read is set

I think you need to disable this one too because it requires network access. We could add like a NO_REMOTE environment variable so all remote tests are automatically disabled without a patch.

have disabled this for the moment before the NO_REMOTE patch lands.

[mojavelinux]

I realized that an rspec tag would be a better solution for the network issue. Please run the test suite as follows:

rspec -t ~network

That will disable any test that requires the network. Note that it does not disable tests that use the local webserver managed by the test itself.

You should no longer have to disable any tests.

[mojavelinux]

@snecklifter Can you confirm that you can run rspec -t ~network without having to disable any tests?

[mojavelinux]

I've merged into master, but will keep open until the fix is confirmed for the v1.5.x branch.

[snecklifter]

@snecklifter Can you confirm that you can run rspec -t ~network without having to disable any tests?

Looks good, thanks!

https://kojipkgs.fedoraproject.org//work/tasks/1645/44371645/build.log

[mojavelinux]

That is fantastic news! Thanks for your work on this @snecklifter. And thanks for the assistance @voxik.