rouge-ruby / rouge

A pure Ruby code highlighter that is compatible with Pygments
https://rouge.jneen.net/
Other
3.33k stars 735 forks source link

WIP: Add snapshot tests for visual samples #1939

Closed nsfisis closed 10 months ago

nsfisis commented 1 year ago

Summary

This PR adds snapshot tests for visual samples (spec/visual/samples/*). It helps contributors to check if their changes cause regressions or not.

Examples

Run rake spec for my PR #1938:

  5) Failure:
Rouge::Lexers::Rouge::Lexers::JSX#test_0003_lexes the sample without changes [/home/ken/src/fork/rouge/spec/lexers_spec.rb:47]:
--- expected
+++ actual
@@ -424,7 +424,7 @@
 Text:  \" \"
 Punctuation:  \"{\"
 Text:  \"<newline>    \"
-Name.Function:  \"super\"
+Keyword:  \"super\"
 Punctuation:  \"();\"
 Text:  \"<newline>    \"
 Keyword:  \"this\"
@@ -513,7 +513,7 @@
 Literal.String.Delimiter:  \"'\"
 Punctuation:  \";\"
 Text:  \"<newline>    \"
-Name.Function:  \"return \"
+Keyword:  \"return \"
 Punctuation:  \"(\"
 Text:  \"<newline>      \"
 Punctuation:  \"<\"

The failure log shows that the PR changes token types of super and return in JavaScript.

How to work

TODO

nsfisis commented 1 year ago
  • Fix potentially flaky tests.
    • Some lexers seem to return slightly different results for each test run.

The snapshot test of Twig sometimes fails.

It is because Twig lexer shares @@keywords variable with its parent class, Jinja. Both class defines keywords method like this:

class Jinja
  def self.keywords
    @@keywords ||= ...
  end
end

class Twig < Jinja
  def self.keywords
    @@keywords ||= ...
  end
end

If Twig.keywords is called before Jinja.keywords, @@keywords is set to Twig's and both class returns the same value. If not, @@keywords is initialized to Jinja's. The snapshot test of Twig fails only if the test of Twig lexer is run after Jinja is.

nsfisis commented 1 year ago
  • Fix potentially flaky tests.
    • Some lexers seem to return slightly different results for each test run.

I found another. The snapshot test of Wollok sometimes fails.

It is because Wollok lexer has a class-scoped local variable, entities, to store entity list, which the lexer distinguishes token types by. Class-scoped local variables are shared between its instances in Ruby. As a result, a token type sometimes change depending on the state of entities.

jneen commented 1 year ago

Good catch. Double-@ class variables shouldn't be used in lexers.

nsfisis commented 10 months ago

I am closing this PR as it would break the CI tests for pending or active PRs, requiring the authors of those PRs to address the CI failures.