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

Include external reporting template for parsed results too #2506

Merged
merged 3 commits into from
Nov 19, 2019

Conversation

marmarek
Copy link
Contributor

Allow branding to set external reporting link to parsed results too.
Since parsed results are all rendered as part of a single template,
module and step id needs to be set directly in a template during
the iteration. And branding template modified to actually use those
values.

I'm not an expert in CSS, and this is the best I can do - the action link is at the bottom right of expanded parsed result text. If anyone know how to put it at the top right (but not increasing height of the collapsed row), that would be more consistent with other places.
You can see how it looks now here: https://openqa.qubes-os.org/tests/4710#step/QubeManagerTest/69

I've included changes to openSUSE branding, but in fact I've tested it on a Qubes OS one (not published yet). BTW is there a way to load branding from an alternative directory?

@marmarek marmarek force-pushed the parser-external-reporting branch 3 times, most recently from 4faa401 to e44c2a7 Compare November 16, 2019 13:27
Copy link
Member

@okurz okurz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @marmarek , welcome back :)

Regarding your question of using a custom branding. You can set a value within /etc/openqa/openqa.ini and you can put branding files within /etc/openqa/templates with the same relative path as within /usr/share/openqa/templates which can override templates or provide some for brandings that do not exist in the main source tree.

templates/test/module_table.html.ep Outdated Show resolved Hide resolved
@marmarek
Copy link
Contributor Author

marmarek commented Nov 16, 2019

The diff coverage is 0%.

That's weird, as my initial commit (see force-push history; EDIT: oh, github shows only diff for the last one...) definitely caused some test failures, so it was executed in some of the tests. Is test coverage measured only on a subset of the tests?

@okurz
Copy link
Member

okurz commented Nov 16, 2019

the external branding is certainly not covered by any coverage tests but your other changes should be covered. I wouldn't worry about it though. I could not find any test failures in your previous commit but only the same coverage missing check that failed there.

@marmarek
Copy link
Contributor Author

The previous failure is here: https://circleci.com/gh/os-autoinst/openQA/6066

templates/test/module_table.html.ep Outdated Show resolved Hide resolved
templates/test/module_table.html.ep Outdated Show resolved Hide resolved
This will be useful for calling bug_report_actions in
module_table.html.ep, where test module and step are not stashed.
Allow branding to set external reporting link to parsed results too.
Since parsed results are all rendered as part of a single template,
module and step id needs to be set directly in a template during
the iteration. And branding template modified to actually use those
values.
@marmarek
Copy link
Contributor Author

Done the changes:

  • moved style to css file (actually didn't need to introduce new class, it was enough to set it for step_actions nested in resborder)
  • added parameters to bug_report_actions, instead of stashing thing inside template

The second one I've done in a generic way (any arguments as passed down to the template). If you prefer to explicitly name just module and step id, let me know.

* Move floating action icons before the text
* Increase padding around action icons a little bit (in consistency
  with help icon)
@Martchus
Copy link
Contributor

@marmarek Thanks for changing it according to my suggestions. I've also added a commit to tweak the markup/styling a little bit. I guess that's more efficient than explaining it (see the commit message for details).

@okurz
Copy link
Member

okurz commented Nov 19, 2019

I reported https://progress.opensuse.org/issues/60035 for the unrelated OBS check failure in plugin_obs_rsync_async

@okurz okurz merged commit 82ed88a into os-autoinst:master Nov 19, 2019
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.

None yet

3 participants