Trouble verifying build history deletion with mock Jobs

classic Classic list List threaded Threaded
28 messages Options
12
Reply | Threaded
Open this post in threaded view
|

Re: Trouble verifying build history deletion with mock Jobs

Jesse Glick-4
On Tue, Jul 9, 2019 at 6:45 PM 'Gavin Mogan' via Jenkins Developers
<[hidden email]> wrote:
> timestamp seems to be a protected item in Run, so subclass!:

Seems like overkill. You can use reflection.

--
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 [hidden email].
To view this discussion on the web visit https://groups.google.com/d/msgid/jenkinsci-dev/CANfRfr3X234w5ZzTYxvjmFr-PD%2BN3E-6sj0EU7H648LJVAkbFQ%40mail.gmail.com.
For more options, visit https://groups.google.com/d/optout.
Reply | Threaded
Open this post in threaded view
|

Re: Trouble verifying build history deletion with mock Jobs

Matt Sicker
In reply to this post by Jesse Glick-4
On Tue, Jul 9, 2019 at 5:20 PM Jesse Glick <[hidden email]> wrote:
>
> On Tue, Jul 9, 2019 at 4:47 PM Ullrich Hafner <[hidden email]> wrote:
> > This topic reminds me of one of my architecture guidelines I’m presenting students in my testing lecture:
> > An application should always provide a way to change (or inject) […]
>
> If you are enumerating design flaws in Jenkins APIs which hurt
> testability, you are going to have a very full syllabus. :-)

We can start with one simple yet permeating issue: an aversion to
inversion of control. I've found that working in pure functional
programming for a short period of time is enough to help one grasp the
level of implicit dependencies you took for granted (e.g., having a
Clock, standard in/out/err or similar streams, other implicit state)
as well as a desire to hide these unnecessary details in a more
natural way.

If you can modify the code you're testing, I'd suggest injecting a
java.time.Clock into classes that are checking the current time. Then
you can trivially mock the Clock instance in your tests.

If you want to do this the "Jenkins Way", throw the Clock into a
singleton that's updated by JenkinsRule like Jenkins' own Jenkins
instance.


--
Matt Sicker
Senior Software Engineer, CloudBees

--
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 [hidden email].
To view this discussion on the web visit https://groups.google.com/d/msgid/jenkinsci-dev/CAEot4oyk_vuWmgOOEjAS1pA9R3tiuBo23cNkAZpmsj%2BXSvyiag%40mail.gmail.com.
For more options, visit https://groups.google.com/d/optout.
Reply | Threaded
Open this post in threaded view
|

Re: Trouble verifying build history deletion with mock Jobs

Benjamin Beggs-3
In reply to this post by Benjamin Beggs-3
Thanks for all the ideas here. I think I've settled on my implementation for this, it's a little different than what's been suggested but I think it's efficient and clean. The ModifiedLogRotator subtracts the daysToKeep integer from the Calendar.DAY_OF_YEAR record. I've added a test class called ageUnitTest() to the plugin logic which adds a double of the daysToKeep integer if it is stubbed to return true (otherwise it is always false). All builds recorded are thereby seen as beyond the max age limit and processed accordingly.

I believe this offers coverage for all logic changes I've introduced with this update.


It is now PR#3, as the repository maintainer decided to merge my previous PR's. I've commented and tried to contact him to tell him that PR#2 needed review and shouldn't have been merged, but I haven't been able to get in touch with him.

--
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 [hidden email].
To view this discussion on the web visit https://groups.google.com/d/msgid/jenkinsci-dev/86d2d274-ae3e-4aa2-bdf5-113e1b27c9e4%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.
Reply | Threaded
Open this post in threaded view
|

Re: Trouble verifying build history deletion with mock Jobs

Martin d'Anjou
For:
to tell him that PR#2 needed review and shouldn't have been merged

Are you able to assign a label to the PR? I have used labels like "do not merge" before.

Martin

--
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 [hidden email].
To view this discussion on the web visit https://groups.google.com/d/msgid/jenkinsci-dev/6180cc33-633e-4fb5-81c1-f6900de78898%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.
Reply | Threaded
Open this post in threaded view
|

Re: Trouble verifying build history deletion with mock Jobs

Gavin Mogan
I've found its good practice to prefix/include "WIP" or "WORK IN PROGRESS" in the title.

Labels are only assignable by committers/triagers

On Thu, Jul 11, 2019 at 12:46 PM martinda <[hidden email]> wrote:
For:
to tell him that PR#2 needed review and shouldn't have been merged

Are you able to assign a label to the PR? I have used labels like "do not merge" before.

Martin

--
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 [hidden email].
To view this discussion on the web visit https://groups.google.com/d/msgid/jenkinsci-dev/6180cc33-633e-4fb5-81c1-f6900de78898%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

--
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 [hidden email].
To view this discussion on the web visit https://groups.google.com/d/msgid/jenkinsci-dev/CAG%3D_DusFtXTEnat9ov6%3DyypqG_fxBt3%2BoxYQN57ooLOYR1EwOA%40mail.gmail.com.
For more options, visit https://groups.google.com/d/optout.
Reply | Threaded
Open this post in threaded view
|

Re: Trouble verifying build history deletion with mock Jobs

Benjamin Beggs-2
This is good advice, I will follow it moving forward. FWIW, the title line of the PR description said that unit tests were currently being written for feature and the last comment made on it mentioned my intention to change the unit testing method with a set of rewrites.

On Thu, Jul 11, 2019 at 3:50 PM 'Gavin Mogan' via Jenkins Developers <[hidden email]> wrote:
I've found its good practice to prefix/include "WIP" or "WORK IN PROGRESS" in the title.

Labels are only assignable by committers/triagers

On Thu, Jul 11, 2019 at 12:46 PM martinda <[hidden email]> wrote:
For:
to tell him that PR#2 needed review and shouldn't have been merged

Are you able to assign a label to the PR? I have used labels like "do not merge" before.

Martin

--
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 [hidden email].
To view this discussion on the web visit https://groups.google.com/d/msgid/jenkinsci-dev/6180cc33-633e-4fb5-81c1-f6900de78898%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

--
You received this message because you are subscribed to a topic in the Google Groups "Jenkins Developers" group.
To unsubscribe from this topic, visit https://groups.google.com/d/topic/jenkinsci-dev/_kNEe9ggaW0/unsubscribe.
To unsubscribe from this group and all its topics, send an email to [hidden email].
To view this discussion on the web visit https://groups.google.com/d/msgid/jenkinsci-dev/CAG%3D_DusFtXTEnat9ov6%3DyypqG_fxBt3%2BoxYQN57ooLOYR1EwOA%40mail.gmail.com.
For more options, visit https://groups.google.com/d/optout.

--
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 [hidden email].
To view this discussion on the web visit https://groups.google.com/d/msgid/jenkinsci-dev/CAMSuSKRH7Ex14_dUDg5MSVTh4X%3DjaGYeiGsWxO1877iKW6CkNw%40mail.gmail.com.
For more options, visit https://groups.google.com/d/optout.
Reply | Threaded
Open this post in threaded view
|

Re: Trouble verifying build history deletion with mock Jobs

Benjamin Beggs-3
In reply to this post by Gavin Mogan
This is good advice, I will follow it moving forward. FWIW, the title line of the PR description said that unit tests were currently being written for feature and the last comment made on it mentioned my intention to change the unit testing method with a set of rewrites.

On Thursday, July 11, 2019 at 3:50:09 PM UTC-4, Gavin Mogan wrote:
I've found its good practice to prefix/include "WIP" or "WORK IN PROGRESS" in the title.

Labels are only assignable by committers/triagers

On Thu, Jul 11, 2019 at 12:46 PM martinda <<a href="javascript:" target="_blank" gdf-obfuscated-mailto="GEtRmzp_DAAJ" rel="nofollow" onmousedown="this.href=&#39;javascript:&#39;;return true;" onclick="this.href=&#39;javascript:&#39;;return true;">martin....@...> wrote:
For:
to tell him that PR#2 needed review and shouldn't have been merged

Are you able to assign a label to the PR? I have used labels like "do not merge" before.

Martin

--
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 <a href="javascript:" target="_blank" gdf-obfuscated-mailto="GEtRmzp_DAAJ" rel="nofollow" onmousedown="this.href=&#39;javascript:&#39;;return true;" onclick="this.href=&#39;javascript:&#39;;return true;">jenkin...@googlegroups.com.
To view this discussion on the web visit <a href="https://groups.google.com/d/msgid/jenkinsci-dev/6180cc33-633e-4fb5-81c1-f6900de78898%40googlegroups.com?utm_medium=email&amp;utm_source=footer" target="_blank" rel="nofollow" onmousedown="this.href=&#39;https://groups.google.com/d/msgid/jenkinsci-dev/6180cc33-633e-4fb5-81c1-f6900de78898%40googlegroups.com?utm_medium\x3demail\x26utm_source\x3dfooter&#39;;return true;" onclick="this.href=&#39;https://groups.google.com/d/msgid/jenkinsci-dev/6180cc33-633e-4fb5-81c1-f6900de78898%40googlegroups.com?utm_medium\x3demail\x26utm_source\x3dfooter&#39;;return true;">https://groups.google.com/d/msgid/jenkinsci-dev/6180cc33-633e-4fb5-81c1-f6900de78898%40googlegroups.com.
For more options, visit <a href="https://groups.google.com/d/optout" target="_blank" rel="nofollow" onmousedown="this.href=&#39;https://groups.google.com/d/optout&#39;;return true;" onclick="this.href=&#39;https://groups.google.com/d/optout&#39;;return true;">https://groups.google.com/d/optout.

--
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 [hidden email].
To view this discussion on the web visit https://groups.google.com/d/msgid/jenkinsci-dev/1a614dbe-ed93-4150-bd50-c271bd13cba2%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.
Reply | Threaded
Open this post in threaded view
|

Re: Trouble verifying build history deletion with mock Jobs

Benjamin Beggs-3
In reply to this post by Benjamin Beggs-2
I have updated my PR#3 with a WORK-IN-PROGRESS titling and mentioned not to merge it in the description.

On Thursday, July 11, 2019 at 4:16:37 PM UTC-4, Benjamin Beggs wrote:
This is good advice, I will follow it moving forward. FWIW, the title line of the PR description said that unit tests were currently being written for feature and the last comment made on it mentioned my intention to change the unit testing method with a set of rewrites.

On Thu, Jul 11, 2019 at 3:50 PM 'Gavin Mogan' via Jenkins Developers <[hidden email]> wrote:
I've found its good practice to prefix/include "WIP" or "WORK IN PROGRESS" in the title.

Labels are only assignable by committers/triagers

On Thu, Jul 11, 2019 at 12:46 PM martinda <[hidden email]> wrote:
For:
to tell him that PR#2 needed review and shouldn't have been merged

Are you able to assign a label to the PR? I have used labels like "do not merge" before.

Martin

--
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 [hidden email].
To view this discussion on the web visit <a href="https://groups.google.com/d/msgid/jenkinsci-dev/6180cc33-633e-4fb5-81c1-f6900de78898%40googlegroups.com?utm_medium=email&amp;utm_source=footer" target="_blank" rel="nofollow" onmousedown="this.href=&#39;https://groups.google.com/d/msgid/jenkinsci-dev/6180cc33-633e-4fb5-81c1-f6900de78898%40googlegroups.com?utm_medium\x3demail\x26utm_source\x3dfooter&#39;;return true;" onclick="this.href=&#39;https://groups.google.com/d/msgid/jenkinsci-dev/6180cc33-633e-4fb5-81c1-f6900de78898%40googlegroups.com?utm_medium\x3demail\x26utm_source\x3dfooter&#39;;return true;">https://groups.google.com/d/msgid/jenkinsci-dev/6180cc33-633e-4fb5-81c1-f6900de78898%40googlegroups.com.
For more options, visit <a href="https://groups.google.com/d/optout" target="_blank" rel="nofollow" onmousedown="this.href=&#39;https://groups.google.com/d/optout&#39;;return true;" onclick="this.href=&#39;https://groups.google.com/d/optout&#39;;return true;">https://groups.google.com/d/optout.

--
You received this message because you are subscribed to a topic in the Google Groups "Jenkins Developers" group.
To unsubscribe from this topic, visit <a href="https://groups.google.com/d/topic/jenkinsci-dev/_kNEe9ggaW0/unsubscribe" target="_blank" rel="nofollow" onmousedown="this.href=&#39;https://groups.google.com/d/topic/jenkinsci-dev/_kNEe9ggaW0/unsubscribe&#39;;return true;" onclick="this.href=&#39;https://groups.google.com/d/topic/jenkinsci-dev/_kNEe9ggaW0/unsubscribe&#39;;return true;">https://groups.google.com/d/topic/jenkinsci-dev/_kNEe9ggaW0/unsubscribe.
To unsubscribe from this group and all its topics, send an email to [hidden email].
To view this discussion on the web visit <a href="https://groups.google.com/d/msgid/jenkinsci-dev/CAG%3D_DusFtXTEnat9ov6%3DyypqG_fxBt3%2BoxYQN57ooLOYR1EwOA%40mail.gmail.com?utm_medium=email&amp;utm_source=footer" target="_blank" rel="nofollow" onmousedown="this.href=&#39;https://groups.google.com/d/msgid/jenkinsci-dev/CAG%3D_DusFtXTEnat9ov6%3DyypqG_fxBt3%2BoxYQN57ooLOYR1EwOA%40mail.gmail.com?utm_medium\x3demail\x26utm_source\x3dfooter&#39;;return true;" onclick="this.href=&#39;https://groups.google.com/d/msgid/jenkinsci-dev/CAG%3D_DusFtXTEnat9ov6%3DyypqG_fxBt3%2BoxYQN57ooLOYR1EwOA%40mail.gmail.com?utm_medium\x3demail\x26utm_source\x3dfooter&#39;;return true;">https://groups.google.com/d/msgid/jenkinsci-dev/CAG%3D_DusFtXTEnat9ov6%3DyypqG_fxBt3%2BoxYQN57ooLOYR1EwOA%40mail.gmail.com.
For more options, visit <a href="https://groups.google.com/d/optout" target="_blank" rel="nofollow" onmousedown="this.href=&#39;https://groups.google.com/d/optout&#39;;return true;" onclick="this.href=&#39;https://groups.google.com/d/optout&#39;;return true;">https://groups.google.com/d/optout.

--
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 [hidden email].
To view this discussion on the web visit https://groups.google.com/d/msgid/jenkinsci-dev/d361ef79-1f4f-4577-8bb3-25df998e43b9%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.
12