Skip to content

Conversation

@dil-gschreiber
Copy link
Contributor

Using characters < or > in test result names, descriptions or details will likely break rendering of the results in the dashboard.

This PR adds proper HTML escaping to avoid this.

Drive-by changes:

  • Use <no name> instead of undefined for tests that have no names
  • Disable eslint rule i18n-text/no-en complaining about using English string literals in code.

@BytewaveMLP
Copy link

@ethomson Apologies for the ping, but is there any chance you could take a look at this? I want to integrate this into a Ruby/RSpec project, which generates a lot of stack traces like:

ActiveRecord::RecordInvalid:
  Validation failed: Name is too long (maximum is 30 characters)
./spec/models/foo_spec.rb:32:in `block (2 levels) in <main>'

Basically all test failures will contain that <main>, and GitHub doesn't handle it well. :(

@BytewaveMLP
Copy link

BytewaveMLP commented Aug 3, 2023

Actually, it looks like this problem is a bit deeper even. We also have quite verbose test failures, which can include things like diffs between models. This leads to output such as this:

Diff:
-[<Foo:0x00007f1a4d46be00
+[<Foo:0x00007f1a4d46be00
- id: 456,
+ id: 123,
- some_attribute: 'some_expected_value'>
+ some_attribute: 'some_value'>

This renders fine in a triple-backtick code block, but GitHub seems to want to parse it as Markdown inside <pre><code> tags, leading to the whole thing being parsed as a list for some reason. I'm not sure if there's a way to tell GitHub to stop parsing Markdown for a given block, or if this could be converted to use Markdown code blocks instead.

update: I did some digging, and it looks like GitHub-Flavored Markdown won't actually disable the Markdown parser inside <pre> blocks UNLESS they appear at the start of their own line. Right now, this action generates something similar to the following:

<tr><td><pre><code>test
details
here</code></pre></td></tr>

For GitHub to properly parse this, however, it needs to appear as

<tr><td>
<pre><code>test
details
here</code></pre></td></tr>

ethomson added a commit that referenced this pull request Dec 12, 2023
GitHub's parser treats '<pre>' at the beginning of a line specially and
will change escaping when it sees it. Cope with that reality.

Fixes #26
@ethomson ethomson merged commit 963fbef into test-summary:main Dec 12, 2023
@ethomson
Copy link
Member

Thanks for the fix and sorry for the delay!

@ethomson
Copy link
Member

Also, thanks @BytewaveMLP for the analysis, I've included your suggestion.

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

Successfully merging this pull request may close these issues.

3 participants