#3 Update to sinatra 2.1.0.
Merged 2 years ago by pvalena. Opened 2 years ago by pvalena.
rpms/ pvalena/rubygem-sinatra rebase  into  rawhide

file modified
+2 -18
@@ -1,18 +1,2 @@ 

- sinatra-1.0.gem

- /sinatra-1.1.2.gem

- /sinatra-1.2.0.gem

- /sinatra-1.2.6.gem

- /sinatra-1.3.2.gem

- /sinatra-1.3.4.gem

- /sinatra-1.3.5.gem

- /sinatra-1.4.3.gem

- /sinatra-1.4.5.gem

- /sinatra-1.4.6.gem

- /sinatra-1.4.7.gem

- /sinatra-1.4.8.gem

- /sinatra-2.0.0-tests.tgz

- /sinatra-2.0.0.gem

- /sinatra-2.0.3-tests.tgz

- /sinatra-2.0.3.gem

- /sinatra-2.0.8.1-test.tar.gz

- /sinatra-2.0.8.1.gem

+ /sinatra-*.gem

+ /sinatra-*-test.tar.gz

@@ -1,25 +0,0 @@ 

- From 750aa3b0de06dad41539bdb402123b5416a3475d Mon Sep 17 00:00:00 2001

- From: Jordan Owens <jkowens@gmail.com>

- Date: Tue, 10 Mar 2020 10:24:05 -0400

- Subject: [PATCH] Fix failing tests

- 

- Rack added support for Multi-part ranges and apparently changed the

- format of cookie expires timestamp format to match specs.

- ---

-  test/static_test.rb |  3 +--

-  1 files changed, 1 insertions(+), 2 deletions(-)

- 

- diff --git a/test/static_test.rb b/test/static_test.rb

- index e8408b14e..1c6cb35e9 100644

- --- a/test/static_test.rb

- +++ b/test/static_test.rb

- @@ -152,8 +152,7 @@ def assert_valid_range(http_range, range, path, file)

-    end

-  

-    it 'correctly ignores syntactically invalid range requests' do

- -    # ...and also ignores multi-range requests, which aren't supported yet

- -    ["bytes=45-40", "bytes=IV-LXVI", "octets=10-20", "bytes=-", "bytes=1-2,3-4"].each do |http_range|

- +    ["bytes=45-40", "bytes=IV-LXVI", "octets=10-20", "bytes=", "bytes=3-1,4-5"].each do |http_range|

-        request = Rack::MockRequest.new(@app)

-        response = request.get("/#{File.basename(__FILE__)}", 'HTTP_RANGE' => http_range)

-  

file modified
+42 -11
@@ -1,29 +1,44 @@ 

  %global gem_name sinatra

  

  %bcond_with bootstrap

+ %bcond_without tilt_integration_tests

  

  Name: rubygem-%{gem_name}

- Version: 2.0.8.1

- Release: 4%{?dist}

+ Version: 2.1.0

+ Release: 1%{?dist}

  Summary: Ruby-based web application framework

  License: MIT

  URL: http://sinatrarb.com/

  Source0: https://rubygems.org/gems/%{gem_name}-%{version}.gem

  # git clone https://github.com/sinatra/sinatra.git && cd sinatra

- # git archive -v -o sinatra-2.0.8.1-test.tar.gz v2.0.8.1 test/

+ # git archive -v -o sinatra-2.1.0-test.tar.gz v2.1.0 test/

  Source1: %{gem_name}-%{version}-test.tar.gz

- # Fix test failure due to Rack 2.2.2 incompatibility.

- # https://github.com/sinatra/sinatra/pull/1605

- Patch0: rubygem-sinatra-2.0.8.1-Fix-failing-tests.patch

  BuildRequires: ruby(release)

  BuildRequires: rubygems-devel

  BuildRequires: ruby >= 2.2.0

  %if %{without bootstrap}

  BuildRequires: rubygem(rack-protection) = %{version}

- BuildRequires: rubygem(tilt)

+ BuildRequires: rubygem(builder)

+ BuildRequires: rubygem(coffee-script)

+ BuildRequires: rubygem(creole)

+ BuildRequires: rubygem(liquid)

  BuildRequires: rubygem(mustermann)

  BuildRequires: rubygem(rack-test)

  BuildRequires: rubygem(minitest) > 5

+ BuildRequires: rubygem(redcarpet)

+ # Tilt is actually required from base_test

+ BuildRequires: rubygem(tilt)

+ BuildRequires: nodejs

+ %if %{with tilt_integration_tests}

+ BuildRequires: rubygem(asciidoctor)

+ BuildRequires: rubygem(rdiscount)

+ BuildRequires: rubygem(kramdown)

+ BuildRequires: rubygem(nokogiri)

+ BuildRequires: rubygem(erubi)

+ BuildRequires: rubygem(haml)

+ BuildRequires: rubygem(slim)

+ BuildRequires: rubygem(sass)

+ %endif

  %endif

  Epoch: 1

  BuildArch: noarch
@@ -44,10 +59,6 @@ 

  %prep

  %setup -q -n %{gem_name}-%{version} -b 1

  

- pushd %{_builddir}

- %patch0 -p1

- popd

- 

  %build

  # Create the gem as gem install only works on a gem file

  gem build ../%{gem_name}-%{version}.gemspec
@@ -77,6 +88,21 @@ 

  # We can't do integration test

  # because we don't ship sinatra-contrib including Sinatra::Runner.

  mv test/integration_test.rb{,.disabled}

+ mv test/integration_async_test.rb{,.disabled}

+ 

+ %if %{without tilt_integration_tests}

+ mv test/asciidoctor_test.rb{,.disabled}

+ %endif

+ 

+ # False positive: inline layouts are rendered differently than expected

+ #-"<h1>THIS. IS. <EM>SPARTA</EM></h1>

+ #+# encoding: ASCII-8BIT

+ #+#    valid: true

+ #+"<h1>THIS. IS. <EM>SPARTA</EM>

+ #+</h1>

+ sed -i '/ layouts" do/ a \

+   skip' test/haml_test.rb

+ 

  # TODO: Is it worth of testing all the possible template engines integration?

  ruby -e 'Dir.glob "./test/*_test.rb", &method(:require)'

  popd
@@ -105,6 +131,11 @@ 

  %{gem_instdir}/sinatra.gemspec

  

  %changelog

+ * Mon Aug 02 2021 Pavel Valena <pvalena@redhat.com> - 1:2.1.0-1

+ - Update to sinatra 2.1.0.

+   Resolves: rhbz#1875978

+   Resolves: rhbz#1970606

+ 

  * Fri Jul 23 2021 Fedora Release Engineering <releng@fedoraproject.org> - 1:2.0.8.1-4

  - Rebuilt for https://fedoraproject.org/wiki/Fedora_35_Mass_Rebuild

  

file modified
+2 -2
@@ -1,2 +1,2 @@ 

- SHA512 (sinatra-2.0.8.1-test.tar.gz) = 3217355e5a8670cf76527bedb422b91993f4abec978c9043e13febeb4c5f9593b72230316dee7fd0db2bb24d0e9f4de9c4ca97e9109161a52d77757a7d436a0f

- SHA512 (sinatra-2.0.8.1.gem) = a63c8bbf468d059ce1e545c4da72208db34ef677141540b31a24a82537c15bbc020c712b9d9eb49470af45b4ce27f65ab21191ee47ebae7c45b9bbd86600ea3e

+ SHA512 (sinatra-2.1.0.gem) = 383c28ea949c2007af813444370f7f4d532bb970d6a69772ac0fafa64f63f1a460966750b10579929550b4e7683b5986cc62c94d98cfb824aa5fa3654e10ef56

+ SHA512 (sinatra-2.1.0-test.tar.gz) = 6135c14790595c5dac8d3b866718278fd4a6417e64062fe5bccf9f97cd488d472297f4a2cea9bf3c4560d0cda00ae5396c01c922881009d15ef47fbd782321c5


To have latest sinatra gem in Fedora.

Koji scratch-build:
https://koji.fedoraproject.org/koji/taskinfo?taskID=73131344
https://kojipkgs.fedoraproject.org/work/tasks/8371/73138371/build.log (enabled tests)

Copr build:
https://copr.fedorainfracloud.org/coprs/build/2356650

Checks:

  • Koji build: ok
  • Tests: ok (the scratch-build was run with bootstrap)
  • Syntax check: ok
  • Reverse dependencies: ok
  • Smoke test: ok (tested manually)
  • rpmlint: ok

Test log: https://git.io/JBQig
gem2rpm diff: https://git.io/JBQi2

Many build dependencies are added. What happened on the upstream?

This failure related to encoding can be fixed by adding LANG or/and LC_ALL?

LANG=C.UTF-8 LC_ALL=C.UTF-8 ruby -e ...

rebased onto 9917dde

2 years ago

Many build dependencies are added. What happened on the upstream?

Nothing that I know of. I've just enabled the integration testing for various packages to find out when it fails earlier.

This si mentioned here:
https://src.fedoraproject.org/rpms/rubygem-sinatra/blob/9917dde1ff35eb2f08a2a5a2e79c606285a3c517/f/rubygem-sinatra.spec#_99

This failure related to encoding can be fixed by adding LANG or/and LC_ALL?

LANG=C.UTF-8 LC_ALL=C.UTF-8 ruby -e ...

I've tried with LC_ALL= .... .. that didn't help.

How about like this?

export LANG=C.UTF-8
export LC_ALL=C.UTF-8
ruby -e ...

For example this bug https://bugs.ruby-lang.org/issues/18002 can be fixed with export LC_ALL=C; make test-all, but not with LC_ALL=C make test-all.

Nothing that I know of. I've just enabled the integration testing for various packages to find out when it fails earlier.

This si mentioned here:
https://src.fedoraproject.org/rpms/rubygem-sinatra/blob/9917dde1ff35eb2f08a2a5a2e79c606285a3c517/f/rubygem-sinatra.spec#_99

it was not enough to understand the context from the comment # TODO: Is it worth of testing all the possible template engines integration?.

Seeing a template test requiring asciidoctor added at sinatra version 1.4.5.
https://github.com/sinatra/sinatra/commit/35e8603d1fa25b8b388875ca743480ec4eb4e1cc

The testing logic is to finish the test with warning even when the asciidoctor gem is not installed on the current rawhide build. This is what I wanted to know. So, you enabled additional tests adding the dependencies.

begin
  require 'asciidoctor'
...
rescue LoadError
  warn "#{$!.to_s}: skipping asciidoc tests"
end

I've just enabled the integration testing for various packages to find out when it fails earlier.

I imagined the integration testing means the following tests. But actually the tests you enabled are not only or not the integration tests.

$ ls test/integration*
test/integration_async_helper.rb  test/integration_helper.rb
test/integration_async_test.rb    test/integration_test.rb

test/integration:
app.rb  rainbows.conf  rainbows.rb

How about adding a comment kind of "Enabled template tests by adding a list of BuildRequires" in the commit message?

And if you enabled all the possible template tests, can we remove the comment # TODO: Is it worth of testing all the possible template engines integration??

I see the following logic.

https://github.com/sinatra/sinatra/blob/0d7e580133a5bb65b05214be7aa9cf195a4698e9/Rakefile#L27-L28

  ENV['LANG'] = 'C'
  ENV.delete 'LC_CTYPE'

So, how about setting the following commands in the spec file?

export LANG=C
unset LC_CTYPE

Did you succeed to pass the tests by bundle exec rake on upstream master? Here is a related code.
https://github.com/sinatra/sinatra/blob/master/.github/workflows/test.yml#L35

Koji scratch-build, without bootstrap, with rack-protection installed; log:
https://kojipkgs.fedoraproject.org/work/tasks/8371/73138371/build.log

I see the following logic.
<snip/>
So, how about setting the following commands in the spec file?

export LANG=C unset LC_CTYPE

No effect as well, but thanks for investigating!

Did you succeed to pass the tests by bundle exec rake on upstream master? Here is a related code.
https://github.com/sinatra/sinatra/blob/master/.github/workflows/test.yml#L35

No, I'm not interested in bundle exec rake output, as this is an integration test for haml, which can be simply caused by different haml version. Also, it's clearly a false positive.

I'd rather disable the all the integration tests that I've just enabled, rather than deeply investigating the false-positives, probably caused just by the difference in buildroot / versions / LANG.

rebased onto 19e3d7c

2 years ago

No, I'm not interested in bundle exec rake output, as this is an integration test for haml, which can be simply caused by different haml version. Also, it's clearly a false positive.

Ah okay, the failure comes from the different version of haml gem. I thought it comes from a difference of the build environment related to encoding between Fedora build system and an environment that the upstream expects.

I'd suggest against including all the template engines. They are mostly tested as optional dependencies of Tilt. Including them (unconditionally) into Sinatra will IMHO just mostly duplicate the Tilt test suite, while significantly blowing up the dependency chain.

I'd suggest against including all the template engines. They are mostly tested as optional dependencies of Tilt. Including them (unconditionally) into Sinatra will IMHO just mostly duplicate the Tilt test suite, while significantly blowing up the dependency chain.

Ok, makes sense, assuming there're no valuable tests not already included in tilt.

These would use "Tilt" (at least in tests):

$ grep -r Tilt -l
mediawiki_test.rb
asciidoctor_test.rb
markdown_test.rb
wlang_test.rb
templates_test.rb
sinatra_test.rb
erb_test.rb
nokogiri_test.rb

So, basically, the list of dependencies to not have installed for tests could like this:

mediawiki
asciidoctor
markdown
wlang
erb
nokogiri

Is there some dependency I've missed?

So, basically, the list of dependencies to not have installed for tests could like this:

How about running the template tests using the list of the dependencies optionally as a default off? It doesn't affect the dependency chain.

%bcond_with template_tests

%if %{with template_tests}
BuildRequires: something
BuildRequires: something
...
%endif

So, basically, the list of dependencies to not have installed for tests could like this:

How about running the template tests using the list of the dependencies optionally as a default off? It doesn't affect the dependency chain.

```
%bcond_with template_tests

%if %{with template_tests}
BuildRequires: something
BuildRequires: something
...
%endif
```

Sure, I'm not against disabling either way.

Just FYI: the following ticket was opened.

F35FailsToInstall: rubygem-sinatra
https://bugzilla.redhat.com/show_bug.cgi?id=1989492

rebased onto 8d71fb5

2 years ago

rebased onto 1ab0da9

2 years ago

rebased onto affad52

2 years ago

rebased onto 23da97f

2 years ago

As Tilt dependency seems to be a core one for the tests (even the base_test), I've opted to leave all the tests enabled.

This can be enhanced at later time.

Pull-Request has been merged by pvalena

2 years ago