> > By the way I would not recommend using your master branch for pull requests > Yep, I was going to do this later on but should have just done it right off the bat.
(by the way please use spaces not tabs) > Hmm I thought I was doing this but I guess not. Will do. > When you write a test you will see that your current implementation does > not work Yea you are right, misunderstanding + oversight on my part. Your clarification makes total sense. I'll make the necessary changes. Thanks for helping me out, I appreciate it. I'm excited to learn more about jenkins and contribute. Patrick On Monday, April 8, 2013 7:36:48 AM UTC-7, Jesse Glick wrote: > > On 04/08/2013 02:21 AM, Patrick McKeown wrote: > > https://github.com/pmaccamp/jenkins/blob/master/ > > By the way I would not recommend using your master branch for pull > requests; it effectively means you can only have one going at once. Much > better to create a branch (git > checkout -b JENKINS-nnnnn-some-description) to hold your changes, so you > can cleanly separate pending PRs from one another and from the trunk. > > (Yes it is possible to move a commit retroactively into a branch, though > it requires some ugly Git commands; Google it.) > > > there are tests located elsewhere than jenkins-core/src/test/java? > > Yes, core/src/test/ has only simple unit tests. Most tests are in a > separate test/ module. > > > HudsonTestCase > > This and its preferred replacement, JenkinsRule (for JUnit 4.x tests), are > in the test/ module. > > When you write a test you will see that your current implementation does > not work. For example, > > public void execute(Runnable arg0) { > SecurityContext executorContext = > SecurityContextHolder.getContext(); > SecurityContextHolder.setContext(initialContext); > try { > service.execute(arg0); > } finally { > SecurityContextHolder.setContext(executorContext); > } > } > > (by the way please use spaces not tabs) is setting the security context on > the calling thread, not the executor thread which is what matters. > initialContext should not be > a field in the service, since no one cares what context was in effect when > the service was created (this may not even have been during an HTTP request > so it is probably > ACL.SYSTEM). Rather your methods should look like > > @Override public void execute(Runnable r) { > service.execute(wrap(r)); > } > > where > > private static Runnable wrap(final Runnable r) { > final SecurityContext callingContext = > SecurityContextHolder.getContext(); > return new Runnable() { > @Override public void run() { > SecurityContext originalExecutorContext = > SecurityContextHolder.getContext(); > SecurityContextHolder.setContext(callingContext); > try { > r.run(); > } finally { > > SecurityContextHolder.setContext(originalExecutorContext); > } > } > }; > } > > and similarly for the Callable methods. > -- You received this message because you are subscribed to the Google Groups "Jenkins Developers" group. To unsubscribe from this group and stop receiving emails from it, send an email to [email protected]. For more options, visit https://groups.google.com/groups/opt_out.
