Support bugrefs in all text results (not just softfailures)

[Martchus]

• Render the result of all text results (not just softfailures) on the server via the rendered_refs_no_shortening helper that is also already used in viewtext.html.
• Remove the condition && title !== 'Soft Failed' so softfailures are no longer a special case; this way the existing test for softfailures will cover these code changes without extending tests

[Martchus]

So with this we'd see the bugref in https://openqa.suse.de/tests/12538408#step/memcg_regression/12 rendered as link. See os-autoinst/os-autoinst-distri-opensuse#17986 (comment) for further context.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 98.31%. Comparing base (f46075b) to head (306e8fc).
Report is 1355 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #5334      +/-   ##
==========================================
- Coverage   98.32%   98.31%   -0.01%     
==========================================
  Files         389      389              
  Lines       37286    37289       +3     
==========================================
+ Hits        36660    36661       +1     
- Misses        626      628       +2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[okurz]

Can you comment on the performance impact. Can you try that with some expensive long kernel test results?

[pevik]

Thank you!

[pevik]

I'm doing some LTP testing.

[pevik]

I saw once > (html escape of >) instead of >, but I cannot find it now. Maybe slow javascript interpretation.

[pevik]

@Martchus @mdoucha I guess there is another html escape instead of interpretation in http://quasar.suse.cz/tests/3038#step/memcontrol01/8:

tst_cgroup.c:865: TCONF: V2 'memory' controller required, but it's mounted on V1

but it should be:

tst_cgroup.c:865: TCONF: V2 'memory' controller required, but it's mounted on V1

[pevik]

@Martchus Fix html escape (not sure if we should fix it in LTP or keep the check for title and just add 'Known').

[pevik]

I wonder if some parameter for javascript + if could determine whether transformation should be done or not.

[Martchus]

I'll clone/check the ltp test and have a look at the raw results. Likely this is a regression by this PR so a change on the LTP side would make no sense. Only if the raw results really contain e.g. ' it would make sense to change this on the LTP side.

[Martchus]

Ah, this problem is actually about "external" text results which now gained that feature as well. That means I must also change the related JavaScript code there. The test results themselves are definitely ok.

[Martchus]

I pushed a version with the escaping problem fixed but unfortunately it turns out to be really problematic to do this kind of rendering server-side. Locally it takes 4 seconds longer to load the mentioned LTP test (which is an increase from 1 second to 5 seconds). The profiler also clearly shows that rendered_refs_no_shortening adds this time. When limiting this feature to normal text results (skipping external text results) it takes only 2 seconds longer. However, I think this is still too much considering there are much bigger tests and OSD is slow anyways.

The alternative would be loading this only on demand like we do for soft failure text results so far. However, this would have the annoyance of a slight loading time when going though text results. This would also cause more traffic/overhead on the server when users are going though text results and again, OSD is slow enough anyways.

This leaves us with implementing this kind of rendering in JavaScript. I've already tried to simply pass the regex to JavaScript and then using it with the JavaScript RegExp class. However, it cannot use that Perl regex so I guess we needed to implement it in JavaScript from scratch (duplicating code). That's also not very nice.

[mdoucha]

The alternative would be loading this only on demand like we do for soft failure text results so far. However, this would have the annoyance of a slight loading time when going though text results. This would also cause more traffic/overhead on the server when users are going though text results and again, OSD is slow enough anyways.

The traffic overhead would still be significantly less than pre-rendering all text details server-side on job page load.

Also note that bugreport links are already rendered server-side even for softfails so you can just drop the regex in Perl and keep only the JS implementation. I have no idea why softfails were kept as an exception when the rendered HTML looks identical whether it's produced in JS or server-side...

[Martchus]

The traffic overhead would still be significantly less than pre-rendering all text details server-side on job page load.

I guess so. Nevertheless the loading time for each individual result would likely be a little bit annoying. At least I'm often going though many text results until I find the one that is actually interesting.

so you can just drop the regex in Perl and keep only the JS implementation

I guess I could at least partially drop the Perl implementation. Annoyingly we also have href_to_bugref on the server side which also uses some of the mappings we define within Perl.

[okurz]

I don't think we can nor should move all that URL parsing to java script

[Martchus]

I think we can. I'm just wondering whether it is worth the effort (as we'd also needed to rewrite tests in Selenium which wouldn't be hard to do but still quite some work).

I actually think we should generally do fronted-related code like deciding how text is rendered via JavaScript. There it is much easier to do things when we actually need them without an AJAX-roundtrip. It also reduced the load on our server. I also generally see no disadvantages on a page like openQA that heavily relies on JavaScript anyways (because we have many features that really couldn't be implemented otherwise).

[okurz]

I also generally see no disadvantages on a page like openQA that heavily relies on JavaScript anyways (because we have many features that really couldn't be implemented otherwise).

I am actually not convinced by that. At time we might want to discuss this further and evaluate on all the alternative approaches

[pevik]

I agree, it's not about web accessibility without javascript (although there might be some improvements) and we probably don't need it much (it helps to SEO, but we delete the results on o3 quite soon anyway to be useful to be stored).

But LTP syscalls tests are a real stress test for a browser and slower laptop/computer. I suspect this is caused by our javascript implementation. Try yourself :).

https://openqa.opensuse.org/tests/3656142
https://openqa.opensuse.org/tests/3656088