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

[Bug]: sassify, utils tests failures #9322

Closed
Apteryks opened this issue Mar 15, 2023 · 8 comments
Closed

[Bug]: sassify, utils tests failures #9322

Apteryks opened this issue Mar 15, 2023 · 8 comments

Comments

@Apteryks
Copy link

Operating System

Guix System

Ruby Version

2.7.4

Jekyll Version

4.3.2

GitHub Pages Version

No response

Expected Behavior

Test should passes.

Current Behavior

$ ruby -w -I"lib:lib:test" test/test_filters*
# -------------------------------------------------------------
# SPECS AND TESTS ARE RUNNING WITH WARNINGS OFF.
# SEE: https://github.com/Shopify/liquid/issues/730
# SEE: https://github.com/jekyll/jekyll/issues/4719
# -------------------------------------------------------------

# Running tests with run options --seed 28148:

...................................F.......................................................................F...........................................................................

Finished tests in 2.173093s, 84.2118 tests/s, 126.0876 assertions/s.


Failure:
TestFilters#test_: filters should sassify with simple string.  [test/test_filters.rb:145]
Minitest::Assertion: --- expected
+++ actual
@@ -1,3 +1,2 @@
-"p {
-  color: #123456;
-}"
+"p { color: #123456; }
+"


Failure:
TestFilters#test_: filters should scssify with simple string.  [test/test_filters.rb:156]
Minitest::Assertion: --- expected
+++ actual
@@ -1,3 +1,2 @@
-"p {
-  color: #123456;
-}"
+"p { color: #123456; }
+"


183 tests, 274 assertions, 2 failures, 0 errors, 0 skips

Relevant log output

No response

Code Sample

No response

@Apteryks
Copy link
Author

The direct dependencies used in the test environment were:

ruby-addressable@2.8.1 ruby-colorator@1.1.0 ruby-cucumber@8.0.0 ruby-em-websocket@0.5.1
+ ruby-httpclient@2.8.3 ruby-i18n@1.7.0 ruby-jekyll-sass-converter@2.1.0
+ ruby-jekyll-test-plugin-malicious@0.2.0 ruby-jekyll-test-plugin@0.2.0 ruby-jekyll-watch@2.1.2
+ ruby-kramdown-parser-gfm@1.1.0 ruby-liquid@4.0.0 ruby-memory-profiler@1.0.0 ruby-mercenary@0.4.0
+ ruby-minitest-profile@0.0.2 ruby-minitest-reporters@1.3.6 ruby-nokogiri@1.13.10 ruby-pathutil@0.16.2
+ ruby-rouge@3.26.1 ruby-rspec-mocks@3.12.4 ruby-rspec@3.12.0 ruby-safe-yaml@1.0.5 ruby-sassc@2.4.0
+ ruby-shoulda@3.5.0 ruby-simplecov@0.22.0 ruby-terminal-table@2.0.0 ruby-tzinfo-data@1.2021.1
+ ruby-tzinfo@2.0.4 ruby-webrick@1.8.1 tzdata@2022a

@Apteryks
Copy link
Author

A similar thing happens for test_sass.rb:

$  ruby -w -I"lib:lib:test" test/test_related_posts.rb ^C
<.3.2.drv-2/source [env]$  ruby -w -I"lib:lib:test" test/test_sass.rb                                    
# -------------------------------------------------------------
# SPECS AND TESTS ARE RUNNING WITH WARNINGS OFF.
# SEE: https://github.com/Shopify/liquid/issues/730
# SEE: https://github.com/jekyll/jekyll/issues/4719
# -------------------------------------------------------------

# Running tests with run options --seed 49245:

.F...

Finished tests in 1.108054s, 4.5124 tests/s, 2.7074 assertions/s.


Failure:
TestSass#test_: importing partials should import SCSS partial.  [test/test_sass.rb:24]
Minitest::Assertion: --- expected
+++ actual
@@ -1,5 +1,3 @@
-".half {
-  width: 50%;
-}
+".half { width: 50%; }
 
 /*# sourceMappingURL=main.css.map */"


5 tests, 3 assertions, 1 failures, 0 errors, 0 skips

@Apteryks Apteryks changed the title [Bug]: sassify tests failures [Bug]: sassify, utils tests failures Mar 15, 2023
@Apteryks
Copy link
Author

Here's one last test that fails for unknown reasons (it's running inside a Linux container which isolates it from the network):

$ ruby -w -I"lib:lib:test" test/test_utils.rb                                 
# -------------------------------------------------------------
# SPECS AND TESTS ARE RUNNING WITH WARNINGS OFF.
# SEE: https://github.com/Shopify/liquid/issues/730
# SEE: https://github.com/jekyll/jekyll/issues/4719
# -------------------------------------------------------------

# Running tests with run options --seed 32300:

........................................................F

Finished tests in 0.863026s, 66.0467 tests/s, 111.2365 assertions/s.


Failure:
TestUtils#test_: Utils::Internet.connected? should return true if there's internet.  [test/test_utils.rb:435]
Minitest::Assertion: Expected false to be truthy.

57 tests, 96 assertions, 1 failures, 0 errors, 0 skips

@ashmaroli
Copy link
Member

Hello @Apteryks, the reason behind this is a change in what we use to convert Sass/Scss into CSS.
Your direct dependencies point to a ruby-jekyll-sass-converter@2.1.0. However, from Jekyll 4.3 onwards, the tests have been altered to be compatible with gem "jekyll-sass-converter", "~> 3.0", which drops use of legacy sass processors to favor the current actively maintained Dart Sass (exposed to Ruby via gem sass-embedded).

The last mentioned test for Utils::Internet.connected? is a flaky test. It will pass on a rerun, provided that the system is able to access the internet.

@Apteryks
Copy link
Author

Hi, and thanks for the precisions. I'll try modernizing the packaging to use sass-embedded instead of jekyll-sass-converter.

As for the last test, the reason it fails in my environment is simple: it runs in a containerized namespace which has no connectivity to the outside (for reproducibility reasons).

@jekyllbot
Copy link
Contributor

This issue has been automatically marked as stale because it has not been commented on for at least two months.

The resources of the Jekyll team are limited, and so we are asking for your help.

If this is a bug and you can still reproduce this error on the latest 3.x-stable or master/main branch, please reply with all of the information you have about it in order to keep the issue open.

If this is a feature request, please consider building it first as a plugin. Jekyll 3 introduced hooks which provide convenient access points throughout the Jekyll build pipeline whereby most needs can be fulfilled. If this is something that cannot be built as a plugin, then please provide more information about why in order to keep this issue open.

This issue will automatically be closed in two months if no further activity occurs. Thank you for all your contributions.

@jekyllbot jekyllbot added the stale Nobody stepped up to work on this issue. label May 16, 2023
@Apteryks
Copy link
Author

Apteryks commented Jul 7, 2023

The only actionable thing I'd see for jekyll on this bug would be to detect if the test environment has networking and skip networking-dependent tests instead of having them fail. Alternatively a test tag 'network' could be used for network-dependent tests, so that they can be skipped by the test runner (I'm assuming this is possible).

Otherwise, feel free to close this. Thanks!

@jekyllbot jekyllbot removed the stale Nobody stepped up to work on this issue. label Jul 7, 2023
@mattr-
Copy link
Member

mattr- commented Jul 8, 2023

Thanks for the report! Assuming you've gotten the sass issues worked out, I'm going to close this as not planned. From my POV, it's not super important to spend time ensuring tests works while isolated from the network. If you do end up fixing this for yourself though, we'd happily merge any PR that would do this.

@mattr- mattr- closed this as not planned Won't fix, can't repro, duplicate, stale Jul 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants