Bugzilla – Full Text Bug Listing
| Summary: | [build] add "Run build" functionality | ||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | z_Archived | Reporter: | Markus Knittig <markus> | ||||||||||||||||||||
| Component: | Mylyn | Assignee: | Markus Knittig <markus> | ||||||||||||||||||||
| Status: | RESOLVED FIXED | QA Contact: | |||||||||||||||||||||
| Severity: | enhancement | ||||||||||||||||||||||
| Priority: | P3 | CC: | steffen.pingel | ||||||||||||||||||||
| Version: | unspecified | Keywords: | plan | ||||||||||||||||||||
| Target Milestone: | 0.7 | ||||||||||||||||||||||
| Hardware: | PC | ||||||||||||||||||||||
| OS: | All | ||||||||||||||||||||||
| Whiteboard: | |||||||||||||||||||||||
| Attachments: |
|
||||||||||||||||||||||
|
Description
Markus Knittig
Created attachment 172296 [details]
Add "Run build" function
Maybe a bit rough, but it works...
Created attachment 172297 [details]
mylyn/context/zip
Thanks. That looks good. I got a couple of conflicts when applying the patch. Could you check that you are synced up with head? A few thoughts about the implementation: * Would it make sense to put the run() method on IBuildPlan? I am not sure what it would it mean to "run" a server or other classes implementing IBuildElement. * Make sure to wrap all network access in RestfulHudsonClient in HudsonOperation classes for proper cancellation support etc. * Can you add a test case that runs a plan and checks that the status changes accordingly? OK, working on fixing that stuff. Putting the run() method in IBuildPlan is a good point, thought about that too, but it's a little bit more complex to get the BuildServerBehaviour there. Not sure it's a good idea to eaxpose this in IBuildServer... It shouldn't be necessary to expose that. Simply cast the result of getServer() to BuildServer: ((BuildServer)getServer()).getBehaviour()... Created attachment 172835 [details]
Runing build
Ah, right, didn't think about that. So, all problems fixed, except that the tests don't work: Running builds needs authentication. Tried the credentials from TestUtil. Doesn't seem to work (or maybe I misinterpret the wiki documentation)...
Created attachment 172836 [details]
mylyn/context/zip
A tests@mylyn.eclipse.org user should now exist on mylyn.eclipse.org with permission to execute builds. I have extended HudsonFixture accordingly but the test still fails. Are you sure that authentication is already supported? I have filed bug 318103 to investigate that which we probably need to resolve first. The patch looks pretty good. There is just one thing that needs to be fixed which is the RunBuildAction. Right now it executes in the UI thread but it should run in a job. Take a look at RefreshOperation for an example how to do that. It would be nice if you could also add the Run action to the view toolbar. Note that I have reformatted the EMF model classes and changed the formatting rules so that it is more consistent with the EMF style. You will probably get a merge conflict but it should be easy enough to resolve. Created attachment 172896 [details]
Running builds
RunBuildAction uses a job, but I did make the implementation a bit nicer. Also add it to the toolbar.
Created attachment 172897 [details]
mylyn/context/zip
(In reply to comment #9) > RunBuildAction uses a job, but I did make the implementation a bit nicer. Also > add it to the toolbar. Thanks. The problem is that the job is not scheduled but run is invoked in the UI thread and can block. I would prefer if we consistently created operations classes which would also make the functionality available to clients and we could add scheduling rules and such later. It will work either way though. Could you fix that and also take a look at MockBuildServerBehavior? I get a compile error after applying the patch. Created attachment 172954 [details]
Running builds
Sorry, missed the test project in the last diff.
Created attachment 172955 [details]
mylyn/context/zip
Created attachment 172957 [details]
mylyn/context/zip
Excellent! I have applied the patch. I tweaked the test a bit since I was getting exceptions when running it. Marking resolved. |