[DISCUSS] should we add some Windows PR checks?

[Owen Nichols <on...@pivotal.io>]

Currently we have zero Windows tests in the PR pipeline, which has led to more than a handful of times this year where a PR passed all its checks then failed the main pipeline once merged.

Adding these 3 jobs to the PR pipeline would have caught the majority of them:
WindowsUnitTestOpenJDK11
WindowsIntegrationTestOpenJDK11
WindowsAcceptanceTestOpenJDK11

The longest of these 3 jobs is about 1/2 hour longer than the current longest PR check.

This is a discussion, not a vote thread.  This might also be a good chance to reflect on whether more or fewer PR checks should be “required”, whether there are other checks we do not have currently but would be nice, the balance between JDK8 and JDK11 checks, or even brainstorm ideas to create additional checks that might not apply to all PRs but could be requested (or attached automatically based on area of code touched).

Re: [DISCUSS] should we add some Windows PR checks?

[Jacob Barrett <jb...@pivotal.io>]

I will toss out there again that there are a number of tests in this set of tests that should have no impact by Windows. If it is truly a Unit tests then windows should note have any impact but we aren’t good about pure unit tests. Integration tests that interact with OS level components, like direct buffers, sockets, files, etc., should be included, all others could be excluded. And argument could be supported that all dunit and acceptance are affected since by nature they all at least use process management, sockets, and filesystems.

Which of the tests have failed I the main pipeline for recent commits. Perhaps we can at least put together a category of smoke tests to run on Windows for PRs.

-Jake

> On Apr 27, 2020, at 2:37 PM, Owen Nichols <on...@pivotal.io> wrote:
> 
> If supporting Windows is costly, I’ll toss out the suggestion that we scale back Windows testing in our regular pipeline as well.  For example, we could run the Windows tests only once a week, once a month, or once per release.
> 
>> On Apr 27, 2020, at 2:00 PM, Robert Houghton <rh...@pivotal.io> wrote:
>> 
>> I would like to *not* include them, until they can take advantage of our
>> run-in-docker behavior, and not be dog slow. Plus, they are *very*
>> expensive instances.
>> 
>> On Mon, Apr 27, 2020 at 1:25 PM Donal Evans <do...@vmware.com> wrote:
>> 
>>> Having just hit this issue myself, and seen it crop up a few times when I
>>> was CIO, I think it would definitely be valuable to add some level of
>>> Windows testing to PR pre-checkin. It's always better to catch issues early
>>> rather than late, and anything that helps keep the pipeline green gets my
>>> approval.
>>> 
>>> Related to this, I personally don't know how to debug a failure and verify
>>> a fix for failing Windows tests locally, so being able to push to a PR
>>> branch and have that all done for me would be very helpful as well.
>>> ________________________________
>>> From: Owen Nichols <on...@pivotal.io>
>>> Sent: Monday, April 27, 2020 1:00 PM
>>> To: dev@geode.apache.org <de...@geode.apache.org>
>>> Subject: [DISCUSS] should we add some Windows PR checks?
>>> 
>>> Currently we have zero Windows tests in the PR pipeline, which has led to
>>> more than a handful of times this year where a PR passed all its checks
>>> then failed the main pipeline once merged.
>>> 
>>> Adding these 3 jobs to the PR pipeline would have caught the majority of
>>> them:
>>> WindowsUnitTestOpenJDK11
>>> WindowsIntegrationTestOpenJDK11
>>> WindowsAcceptanceTestOpenJDK11
>>> 
>>> The longest of these 3 jobs is about 1/2 hour longer than the current
>>> longest PR check.
>>> 
>>> This is a discussion, not a vote thread.  This might also be a good chance
>>> to reflect on whether more or fewer PR checks should be “required”, whether
>>> there are other checks we do not have currently but would be nice, the
>>> balance between JDK8 and JDK11 checks, or even brainstorm ideas to create
>>> additional checks that might not apply to all PRs but could be requested
>>> (or attached automatically based on area of code touched).
>>> 
> 

Re: [DISCUSS] should we add some Windows PR checks?

[Owen Nichols <on...@pivotal.io>]

If supporting Windows is costly, I’ll toss out the suggestion that we scale back Windows testing in our regular pipeline as well.  For example, we could run the Windows tests only once a week, once a month, or once per release.

> On Apr 27, 2020, at 2:00 PM, Robert Houghton <rh...@pivotal.io> wrote:
> 
> I would like to *not* include them, until they can take advantage of our
> run-in-docker behavior, and not be dog slow. Plus, they are *very*
> expensive instances.
> 
> On Mon, Apr 27, 2020 at 1:25 PM Donal Evans <do...@vmware.com> wrote:
> 
>> Having just hit this issue myself, and seen it crop up a few times when I
>> was CIO, I think it would definitely be valuable to add some level of
>> Windows testing to PR pre-checkin. It's always better to catch issues early
>> rather than late, and anything that helps keep the pipeline green gets my
>> approval.
>> 
>> Related to this, I personally don't know how to debug a failure and verify
>> a fix for failing Windows tests locally, so being able to push to a PR
>> branch and have that all done for me would be very helpful as well.
>> ________________________________
>> From: Owen Nichols <on...@pivotal.io>
>> Sent: Monday, April 27, 2020 1:00 PM
>> To: dev@geode.apache.org <de...@geode.apache.org>
>> Subject: [DISCUSS] should we add some Windows PR checks?
>> 
>> Currently we have zero Windows tests in the PR pipeline, which has led to
>> more than a handful of times this year where a PR passed all its checks
>> then failed the main pipeline once merged.
>> 
>> Adding these 3 jobs to the PR pipeline would have caught the majority of
>> them:
>> WindowsUnitTestOpenJDK11
>> WindowsIntegrationTestOpenJDK11
>> WindowsAcceptanceTestOpenJDK11
>> 
>> The longest of these 3 jobs is about 1/2 hour longer than the current
>> longest PR check.
>> 
>> This is a discussion, not a vote thread.  This might also be a good chance
>> to reflect on whether more or fewer PR checks should be “required”, whether
>> there are other checks we do not have currently but would be nice, the
>> balance between JDK8 and JDK11 checks, or even brainstorm ideas to create
>> additional checks that might not apply to all PRs but could be requested
>> (or attached automatically based on area of code touched).
>> 

Re: [DISCUSS] should we add some Windows PR checks?

[Robert Houghton <rh...@pivotal.io>]

I would like to *not* include them, until they can take advantage of our
run-in-docker behavior, and not be dog slow. Plus, they are *very*
expensive instances.

On Mon, Apr 27, 2020 at 1:25 PM Donal Evans <do...@vmware.com> wrote:

> Having just hit this issue myself, and seen it crop up a few times when I
> was CIO, I think it would definitely be valuable to add some level of
> Windows testing to PR pre-checkin. It's always better to catch issues early
> rather than late, and anything that helps keep the pipeline green gets my
> approval.
>
> Related to this, I personally don't know how to debug a failure and verify
> a fix for failing Windows tests locally, so being able to push to a PR
> branch and have that all done for me would be very helpful as well.
> ________________________________
> From: Owen Nichols <on...@pivotal.io>
> Sent: Monday, April 27, 2020 1:00 PM
> To: dev@geode.apache.org <de...@geode.apache.org>
> Subject: [DISCUSS] should we add some Windows PR checks?
>
> Currently we have zero Windows tests in the PR pipeline, which has led to
> more than a handful of times this year where a PR passed all its checks
> then failed the main pipeline once merged.
>
> Adding these 3 jobs to the PR pipeline would have caught the majority of
> them:
> WindowsUnitTestOpenJDK11
> WindowsIntegrationTestOpenJDK11
> WindowsAcceptanceTestOpenJDK11
>
> The longest of these 3 jobs is about 1/2 hour longer than the current
> longest PR check.
>
> This is a discussion, not a vote thread.  This might also be a good chance
> to reflect on whether more or fewer PR checks should be “required”, whether
> there are other checks we do not have currently but would be nice, the
> balance between JDK8 and JDK11 checks, or even brainstorm ideas to create
> additional checks that might not apply to all PRs but could be requested
> (or attached automatically based on area of code touched).
>

Re: [DISCUSS] should we add some Windows PR checks?

[Donal Evans <do...@vmware.com>]

Having just hit this issue myself, and seen it crop up a few times when I was CIO, I think it would definitely be valuable to add some level of Windows testing to PR pre-checkin. It's always better to catch issues early rather than late, and anything that helps keep the pipeline green gets my approval.

Related to this, I personally don't know how to debug a failure and verify a fix for failing Windows tests locally, so being able to push to a PR branch and have that all done for me would be very helpful as well.
________________________________
From: Owen Nichols <on...@pivotal.io>
Sent: Monday, April 27, 2020 1:00 PM
To: dev@geode.apache.org <de...@geode.apache.org>
Subject: [DISCUSS] should we add some Windows PR checks?

Currently we have zero Windows tests in the PR pipeline, which has led to more than a handful of times this year where a PR passed all its checks then failed the main pipeline once merged.

Adding these 3 jobs to the PR pipeline would have caught the majority of them:
WindowsUnitTestOpenJDK11
WindowsIntegrationTestOpenJDK11
WindowsAcceptanceTestOpenJDK11

The longest of these 3 jobs is about 1/2 hour longer than the current longest PR check.

This is a discussion, not a vote thread.  This might also be a good chance to reflect on whether more or fewer PR checks should be “required”, whether there are other checks we do not have currently but would be nice, the balance between JDK8 and JDK11 checks, or even brainstorm ideas to create additional checks that might not apply to all PRs but could be requested (or attached automatically based on area of code touched).