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

Add a display of subtest in TAP Extended Test Results view #18

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

guilcy
Copy link

@guilcy guilcy commented Jul 26, 2017

I modified the jelly code to render one level of subtest when flatten result not checked.
Unfortunately I did not succeed in having a recursive display all the subtests (I am newbe in jelly), even if it may become very difficult to read, I would like to improve this (lot of space is lost in the indentation methode I used, and the number of subtest is now in the Description column.
Another possibility could be to display subtests only after click on the parent.

If flatten result is checked the subtests are rendered the old way.

@kinow kinow changed the title Add a display of subtest in TAP Extended Test Results view WIP Add a display of subtest in TAP Extended Test Results view Jul 28, 2017
@kinow kinow changed the title WIP Add a display of subtest in TAP Extended Test Results view [WIP] Add a display of subtest in TAP Extended Test Results view Jul 28, 2017
@kinow
Copy link
Member

kinow commented Jul 28, 2017

Thanks @guilcy !!! I noticed you are still working on it, so added a [WIP] marker. Feel free to remove it once you are done, and I will know we can review it.

Thanks!!!
Bruno

@guilcy
Copy link
Author

guilcy commented Jul 28, 2017

Well, yes I did some new motifications in my fork, I though this would not be visible in the pull request (I am new with using the tool). Basically even if the rendering is much nicer with the last changes (and the coding better) , as I did not find how to make recursivity into Jelly, I replicate one Jelly script 5 times to match the sub test levels ( due to this it is limited to 5 levels of subtest), this is ugly (I would prefer a nice Jelly trick instead), or a another solution would reside in changes in the java (add an interface) to render a testSet, flattend, with indent information in each tapLines (or tapElement).
Do you know if it is possible to have Jelly script call itself to avoid the 5 scripts chain ?

@kinow
Copy link
Member

kinow commented Jul 28, 2017

Well, yes I did some new motifications in my fork, I though this would not be visible in the pull request (I am new with using the tool)

You are doing great! Your pull request is not hard to review (i.e. you are not changing things outside the scope of your change like formatting, code style, etc). The code looks good. I'm a bit concerned about the limitations you pointed, but happy to compromise if it doesn't impact users.

this is ugly (I would prefer a nice Jelly trick instead)

I can review the Jelly later, or we can look at the Groovy examples now. I wrote the code some years ago (even before publishing, I was using in a company while writing tap4j), so there are probably a few parts of the code that can be improved (I know for certain that memory usage has to be reduced, for that someone needs to rewrite the junit integration to not load so many objects in memory... but digressing here.. sorry)

Do you know if it is possible to have Jelly script call itself to avoid the 5 scripts chain ?

Not sure. Normally I would have Jelly file A, calling Jelly file B in a loop. If A needed to call A... well... I am not sure what would happen :-)

@guilcy guilcy changed the title [WIP] Add a display of subtest in TAP Extended Test Results view Add a display of subtest in TAP Extended Test Results view Jul 28, 2017
@guilcy
Copy link
Author

guilcy commented Jul 28, 2017

The need is really Jelly A call Jelly A... and this is causing a crash inside Jenkins.
I am happy with the current rendering, and will not change further for now, thus I have removed the WIP, so that people can review the code.
Let me know how to improve.

@guilcy
Copy link
Author

guilcy commented Sep 13, 2017

I have pushed a new commit to fix a typo in jelly script which cause problem on recent jenkins (but works fine on my dev/debug setup).
By the way I did not succeed in setting up eclipse on the latest jenkins.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants