-
Notifications
You must be signed in to change notification settings - Fork 14
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
diff between two stack traces #27
Conversation
import io.jenkins.plugins.report.jtreg.model.Suite; | ||
import io.jenkins.plugins.report.jtreg.model.Test; | ||
import io.jenkins.plugins.report.jtreg.model.TestOutput; | ||
import io.jenkins.plugins.report.jtreg.model.*; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please do not use * imports unless very necesary, which this is not.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ahh, I will change that, my IDE did that automatically
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
disable that :) its evil :)
major idea... Isnt worthy to do a separate backend for this? That would be new module - as is diff and comparator, and it will be new service endpoint. ANy shared code can go t the lib, and the only shared switch is |
Thanx a lot for rebasing. Few notes. please drop df848f4#diff-4c0b2eb7d59022ddbf95f75466f335e65d68ad916feabc1d2dae42315960603bR99 during implementation of links. It is there jsut for you to see how to use that. |
I can probably do that. I don't think that it would break anything and it should be easy hopefully
ok, I will take a look at that |
of course you can do that :) But do you agree... do you think it is good idea? I htink yes, but it is just 50% of current project crew :) |
I think it's a good idea, it makes sense to have it separately. If nothing else, then at least the code will be clearer :) |
yah, the number of parametters may grow pretty much... unluckily, the haow to compare switches are shared with the comparator, pls, reuse the code. |
The dealing with shared code wil be quite a pain and burden. but afaik still worthy. |
What switches do you mean? There are two, which both use:
So should I move them to lib (even the parsing)? |
Yes please. The formatting is that type:howMuch thing? |
No, that is another thing, but good catch, it also needs to be moved there. |
And --path, --jenkins-url .. and so on :( |
Do you recall how our discussion or removals of parts of the trace ended? HAve we agreed on some |
…he arg parsing object oriented
…k traces (was a part of comparator before)
I extracted the common argument parsing (and everything it needs) from comparator to the lib module. I also tried to make the argument parsing object oriented now. Apart from that, I created a new module called TraceDiff with the extracted stack trace diff tool from comparator. Since the service module is not setup yet for this new tool, I haven't created any links yet :( It can be run by:
|
You had merged in master:( Please do not do that, rebase on top of master. |
It should be easy to enbale the service . Will you, or should I? |
Thanx! The synax is surprisngly friendly at the end! |
Plese add at least one test to th new module. |
Even if we push under this name (traceDiff) we have to rename. My idea is:
wdyt? When to do so? |
Sorry for that, I didn't notice
I can try to take a look at that :)
ok, will do
I can do that in this PR if you want that |
== go for it, at least from scope of renaming the modules and variaous pom artifacts. Me as downstream, will have a bit of fun with that, but it will be worthy. Also the links pointing to the current diff on hydra will needaadjsut (have we already made them adjsutable? If not, it will be mandatory work to do togehter with traceDiff links) The name diff is used in the service wrapper scripts, and that will need to follow that too. Also it is used in the service endpint name, ut I'm +1 to fix that sooner rather then later. So feel free to procedd with reanming up to where you feel safe, and then lts stop, merge, and continue separately. |
The renaming of the modules is (hopefully correctly without any problems) done.
Also, the endpoint of the former diff's service was changed from |
Jsut recall me, current "list" is my old tool. Curren report-jtreg-comparator is your new comaprator which can comapre test results and traces. and current diff is the newest module, which is showing the diff between traes. oook? |
If so, then lets keep it like that. Its good enough. |
yes, exactly |
ty! |
Still work in progress, I need to set the links in the table to this first.
The comparator now supports creating a diff between two stack traces. It can be shown in multiple formats:
There are three new command line switches:
--trace-from jobName:buildNumber
-- for setting the first build--trace-to jobName:buildNumber
-- for setting the second build--diff-format patch/inline/sidebyside
-- for choosing the output format of the diffIt takes the set of tests to create the diff from the
--exact-tests ".*"
argument, so you also need to set it.