Re: [PROPOSAL] Add windows jobs to PR checks
Requesting Windows checks on an individual PR is now as simple as adding a "windows" label. Labels can be added in GitHub (just below the list of reviewers). The best time to add this is when creating the PR (if adding this label to an existing PR, you'll need to push another commit or an empty commit before it will "see" the new label). A huge thanks to new committer @sabbey37 for making this possible! On 6/25/20, 3:51 PM, "Kirk Lund" wrote: How about another option: 6) Provide someway to file a secondary/optional Pull Request that just runs the Windows tests. This 2nd PR would never go anywhere (just get closed after reviewing the test results) and it could even be on a fork of Apache Geode to keep it from polluting the main PR section for Apache Geode. Submitting this Windows PR would be optional and I would probably only submit certain kinds of changes or tests to run on the Windows tests. On Thu, Jun 25, 2020 at 1:51 PM Anilkumar Gingade wrote: > Looking at the cost and value derived; My vote is with current/existing > process (not running for every PR). > > On 6/25/20, 11:39 AM, "Mark Hanson" wrote: > > I support adding it in, but I think the time wasted is less than you > think. I think for me the most important thing is finding an issue when it > is put in. > > I think the current way is actually faster and more efficient, because > every PR doesn’t have to wait the 4 hours and in reality the number is of > windows failures is lower than the number of linux failures. > > Just a thought. > > Thanks, > Mark > > > > On Jun 25, 2020, at 11:30 AM, Jianxia Chen > wrote: > > > > +1 to add Windows tests to the PR pipeline. It may take longer time > to run > > (up to 4 hours). But consider the time wasted on reverting, fixing > and > > resubmitting, if there is a failure after merging to the develop > branch. It > > is better to add the Windows tests to the PR pipeline. We can > reevaluate > > and optimize the pipeline if the long running time is truly a > concern. > > > > On Thu, Jun 25, 2020 at 9:29 AM Kirk Lund wrote: > > > >> I merged some new AcceptanceTests to develop after having my PR go > GREEN. > >> But now these tests are failing in Windows. > >> > >> I'd like to propose that we add the Windows jobs to our PR checks > if we > >> plan to keep testing on Windows in CI. > >> > >> Please vote or discuss. > >> > >> Thanks, > >> Kirk > >> > > >
Re: [PROPOSAL] Add windows jobs to PR checks
What I am looking for is a script like following: ./regression -Z deploy \ -n \ -o \ -g \ -u \ -t \ -k \ -F \ Example: ./regression deploy -n 10 -o centos7 \ -g ~/gemfire/closed/pivotalgf-assembly/build/distributions/pivotal-gemfire-regression-0.0.0.tgz \ -k ~/.ssh/id_rsa.pub -u johndoe -t storageteam myregression ``` Operating systems to choose from are: * centos7 * rhel7 * ubuntu14* * ubuntu16* * sles12* * sles11* * windows ^ > We use to have a script call "precheckin". I forgot if we can select operating system like above "regression" script. On 6/25/20, 4:09 PM, "Xiaojian Zhou" wrote: I vote to is also with current/existing process (not running for every PR). We can create an on-request prechecking running on windows machine like what we did for running some regression tests, if someone really need to run it on windows (Actually, I'd love to have this tool) On 6/25/20, 1:52 PM, "Anilkumar Gingade" wrote: Looking at the cost and value derived; My vote is with current/existing process (not running for every PR). On 6/25/20, 11:39 AM, "Mark Hanson" wrote: I support adding it in, but I think the time wasted is less than you think. I think for me the most important thing is finding an issue when it is put in. I think the current way is actually faster and more efficient, because every PR doesn’t have to wait the 4 hours and in reality the number is of windows failures is lower than the number of linux failures. Just a thought. Thanks, Mark > On Jun 25, 2020, at 11:30 AM, Jianxia Chen wrote: > > +1 to add Windows tests to the PR pipeline. It may take longer time to run > (up to 4 hours). But consider the time wasted on reverting, fixing and > resubmitting, if there is a failure after merging to the develop branch. It > is better to add the Windows tests to the PR pipeline. We can reevaluate > and optimize the pipeline if the long running time is truly a concern. > > On Thu, Jun 25, 2020 at 9:29 AM Kirk Lund wrote: > >> I merged some new AcceptanceTests to develop after having my PR go GREEN. >> But now these tests are failing in Windows. >> >> I'd like to propose that we add the Windows jobs to our PR checks if we >> plan to keep testing on Windows in CI. >> >> Please vote or discuss. >> >> Thanks, >> Kirk >>
Re: [PROPOSAL] Add windows jobs to PR checks
I vote to is also with current/existing process (not running for every PR). We can create an on-request prechecking running on windows machine like what we did for running some regression tests, if someone really need to run it on windows (Actually, I'd love to have this tool) On 6/25/20, 1:52 PM, "Anilkumar Gingade" wrote: Looking at the cost and value derived; My vote is with current/existing process (not running for every PR). On 6/25/20, 11:39 AM, "Mark Hanson" wrote: I support adding it in, but I think the time wasted is less than you think. I think for me the most important thing is finding an issue when it is put in. I think the current way is actually faster and more efficient, because every PR doesn’t have to wait the 4 hours and in reality the number is of windows failures is lower than the number of linux failures. Just a thought. Thanks, Mark > On Jun 25, 2020, at 11:30 AM, Jianxia Chen wrote: > > +1 to add Windows tests to the PR pipeline. It may take longer time to run > (up to 4 hours). But consider the time wasted on reverting, fixing and > resubmitting, if there is a failure after merging to the develop branch. It > is better to add the Windows tests to the PR pipeline. We can reevaluate > and optimize the pipeline if the long running time is truly a concern. > > On Thu, Jun 25, 2020 at 9:29 AM Kirk Lund wrote: > >> I merged some new AcceptanceTests to develop after having my PR go GREEN. >> But now these tests are failing in Windows. >> >> I'd like to propose that we add the Windows jobs to our PR checks if we >> plan to keep testing on Windows in CI. >> >> Please vote or discuss. >> >> Thanks, >> Kirk >>
Re: [PROPOSAL] Add windows jobs to PR checks
How about another option: 6) Provide someway to file a secondary/optional Pull Request that just runs the Windows tests. This 2nd PR would never go anywhere (just get closed after reviewing the test results) and it could even be on a fork of Apache Geode to keep it from polluting the main PR section for Apache Geode. Submitting this Windows PR would be optional and I would probably only submit certain kinds of changes or tests to run on the Windows tests. On Thu, Jun 25, 2020 at 1:51 PM Anilkumar Gingade wrote: > Looking at the cost and value derived; My vote is with current/existing > process (not running for every PR). > > On 6/25/20, 11:39 AM, "Mark Hanson" wrote: > > I support adding it in, but I think the time wasted is less than you > think. I think for me the most important thing is finding an issue when it > is put in. > > I think the current way is actually faster and more efficient, because > every PR doesn’t have to wait the 4 hours and in reality the number is of > windows failures is lower than the number of linux failures. > > Just a thought. > > Thanks, > Mark > > > > On Jun 25, 2020, at 11:30 AM, Jianxia Chen > wrote: > > > > +1 to add Windows tests to the PR pipeline. It may take longer time > to run > > (up to 4 hours). But consider the time wasted on reverting, fixing > and > > resubmitting, if there is a failure after merging to the develop > branch. It > > is better to add the Windows tests to the PR pipeline. We can > reevaluate > > and optimize the pipeline if the long running time is truly a > concern. > > > > On Thu, Jun 25, 2020 at 9:29 AM Kirk Lund wrote: > > > >> I merged some new AcceptanceTests to develop after having my PR go > GREEN. > >> But now these tests are failing in Windows. > >> > >> I'd like to propose that we add the Windows jobs to our PR checks > if we > >> plan to keep testing on Windows in CI. > >> > >> Please vote or discuss. > >> > >> Thanks, > >> Kirk > >> > > >
Re: [PROPOSAL] Add windows jobs to PR checks
Looking at the cost and value derived; My vote is with current/existing process (not running for every PR). On 6/25/20, 11:39 AM, "Mark Hanson" wrote: I support adding it in, but I think the time wasted is less than you think. I think for me the most important thing is finding an issue when it is put in. I think the current way is actually faster and more efficient, because every PR doesn’t have to wait the 4 hours and in reality the number is of windows failures is lower than the number of linux failures. Just a thought. Thanks, Mark > On Jun 25, 2020, at 11:30 AM, Jianxia Chen wrote: > > +1 to add Windows tests to the PR pipeline. It may take longer time to run > (up to 4 hours). But consider the time wasted on reverting, fixing and > resubmitting, if there is a failure after merging to the develop branch. It > is better to add the Windows tests to the PR pipeline. We can reevaluate > and optimize the pipeline if the long running time is truly a concern. > > On Thu, Jun 25, 2020 at 9:29 AM Kirk Lund wrote: > >> I merged some new AcceptanceTests to develop after having my PR go GREEN. >> But now these tests are failing in Windows. >> >> I'd like to propose that we add the Windows jobs to our PR checks if we >> plan to keep testing on Windows in CI. >> >> Please vote or discuss. >> >> Thanks, >> Kirk >>
Re: [PROPOSAL] Add windows jobs to PR checks
It's been a couple of years since Sai and I tried (but failed) to dockerize the tests. I'm sure docker support has improved and it might be worth trying that again. --Jens On 6/25/20, 10:08 AM, "Jacob Barrett" wrote: > On Jun 25, 2020, at 10:01 AM, Jinmei Liao wrote: > > +1, what was the reason for it not being included the PR before? The Windows integration and acceptance tests take a very long time to run because we can’t dockerize and parallelize them. They have also be very flaky in the past.
Re: [PROPOSAL] Add windows jobs to PR checks
I support adding it in, but I think the time wasted is less than you think. I think for me the most important thing is finding an issue when it is put in. I think the current way is actually faster and more efficient, because every PR doesn’t have to wait the 4 hours and in reality the number is of windows failures is lower than the number of linux failures. Just a thought. Thanks, Mark > On Jun 25, 2020, at 11:30 AM, Jianxia Chen wrote: > > +1 to add Windows tests to the PR pipeline. It may take longer time to run > (up to 4 hours). But consider the time wasted on reverting, fixing and > resubmitting, if there is a failure after merging to the develop branch. It > is better to add the Windows tests to the PR pipeline. We can reevaluate > and optimize the pipeline if the long running time is truly a concern. > > On Thu, Jun 25, 2020 at 9:29 AM Kirk Lund wrote: > >> I merged some new AcceptanceTests to develop after having my PR go GREEN. >> But now these tests are failing in Windows. >> >> I'd like to propose that we add the Windows jobs to our PR checks if we >> plan to keep testing on Windows in CI. >> >> Please vote or discuss. >> >> Thanks, >> Kirk >>
Re: [PROPOSAL] Add windows jobs to PR checks
+1 to add Windows tests to the PR pipeline. It may take longer time to run (up to 4 hours). But consider the time wasted on reverting, fixing and resubmitting, if there is a failure after merging to the develop branch. It is better to add the Windows tests to the PR pipeline. We can reevaluate and optimize the pipeline if the long running time is truly a concern. On Thu, Jun 25, 2020 at 9:29 AM Kirk Lund wrote: > I merged some new AcceptanceTests to develop after having my PR go GREEN. > But now these tests are failing in Windows. > > I'd like to propose that we add the Windows jobs to our PR checks if we > plan to keep testing on Windows in CI. > > Please vote or discuss. > > Thanks, > Kirk >
Re: [PROPOSAL] Add windows jobs to PR checks
In principal, +1 for adding them. But if it is gating or not, is determined by how much extra time we now have to add to waiting for a PR build to complete. Is there any way that we could improve the time testing time of these? —Udo On Jun 25, 2020, 11:05 AM -0700, Bruce Schuchardt , wrote: If they take a very long time to run, how about adding them but not requiring them to pass? On 6/25/20, 10:08 AM, "Jacob Barrett" wrote: On Jun 25, 2020, at 10:01 AM, Jinmei Liao wrote: +1, what was the reason for it not being included the PR before? The Windows integration and acceptance tests take a very long time to run because we can’t dockerize and parallelize them. They have also be very flaky in the past.
Re: [PROPOSAL] Add windows jobs to PR checks
If they take a very long time to run, how about adding them but not requiring them to pass? On 6/25/20, 10:08 AM, "Jacob Barrett" wrote: > On Jun 25, 2020, at 10:01 AM, Jinmei Liao wrote: > > +1, what was the reason for it not being included the PR before? The Windows integration and acceptance tests take a very long time to run because we can’t dockerize and parallelize them. They have also be very flaky in the past.
Re: [PROPOSAL] Add windows jobs to PR checks
This is going to hurt timewise, but +1. > On Jun 25, 2020, at 10:11 AM, Kirk Lund wrote: > > Another option: > > (5) introduce a new staging branch that PRs merge directly to. Windows > testing occurs on this staging branch. Then any PRs that pass on the > staging branch can then be merged or cherry-picked to develop. > > On Thu, Jun 25, 2020 at 10:08 AM Jacob Barrett wrote: > >> >> >>> On Jun 25, 2020, at 10:01 AM, Jinmei Liao wrote: >>> >>> +1, what was the reason for it not being included the PR before? >> >> The Windows integration and acceptance tests take a very long time to run >> because we can’t dockerize and parallelize them. They have also be very >> flaky in the past. >> >>
Re: [PROPOSAL] Add windows jobs to PR checks
Another option: (5) introduce a new staging branch that PRs merge directly to. Windows testing occurs on this staging branch. Then any PRs that pass on the staging branch can then be merged or cherry-picked to develop. On Thu, Jun 25, 2020 at 10:08 AM Jacob Barrett wrote: > > > > On Jun 25, 2020, at 10:01 AM, Jinmei Liao wrote: > > > > +1, what was the reason for it not being included the PR before? > > The Windows integration and acceptance tests take a very long time to run > because we can’t dockerize and parallelize them. They have also be very > flaky in the past. > >
Re: [PROPOSAL] Add windows jobs to PR checks
+1 I recently ran into some Windows failures related to test ordering and Integration tests not properly cleaning up after themselves (totally unrelated to my changes) after merging a PR. If the PR checks had shown these failures, the underlying issue could have been addressed before merging my changes and avoided the need to revert. From: Jinmei Liao Sent: Thursday, June 25, 2020 10:01 AM To: dev@geode.apache.org Subject: Re: [PROPOSAL] Add windows jobs to PR checks +1, what was the reason for it not being included the PR before? From: Dick Cavender Sent: Thursday, June 25, 2020 9:54 AM To: dev@geode.apache.org Subject: RE: [PROPOSAL] Add windows jobs to PR checks +1 -Original Message- From: Owen Nichols Sent: Thursday, June 25, 2020 9:38 AM To: dev@geode.apache.org Subject: Re: [PROPOSAL] Add windows jobs to PR checks +1 for adding all JDK11 Windows tests to PR pipeline. On 6/25/20, 9:29 AM, "Kirk Lund" wrote: I merged some new AcceptanceTests to develop after having my PR go GREEN. But now these tests are failing in Windows. I'd like to propose that we add the Windows jobs to our PR checks if we plan to keep testing on Windows in CI. Please vote or discuss. Thanks, Kirk
Re: [PROPOSAL] Add windows jobs to PR checks
> On Jun 25, 2020, at 10:01 AM, Jinmei Liao wrote: > > +1, what was the reason for it not being included the PR before? The Windows integration and acceptance tests take a very long time to run because we can’t dockerize and parallelize them. They have also be very flaky in the past.
Re: [PROPOSAL] Add windows jobs to PR checks
+1 to add Windows jobs to PR checks (1) I know there are some folks who may be resistant to this for good reasons, but the problem is that what we are currently doing is untenable and wasteful. Every other week, I hit this and then waste a day reverting, fixing, resubmitting. I don't want to submit more PRs with tests until this is actually resolved because I'm sick of this problem. In order of my preferences: 1) add Windows jobs to PR checks 2) remove Windows jobs from CI if we don't test them in PR (I *NEVER* want to see a *GREEN* PR that then breaks CI) 3) add some new complicated process in which we manually or automatically trigger Windows jobs against a PR *BEFORE* we consider merging it to develop 4) stop writing new tests or editing existing tests that might fail on Windows Thanks, Kirk On Thu, Jun 25, 2020 at 9:29 AM Kirk Lund wrote: > I merged some new AcceptanceTests to develop after having my PR go GREEN. > But now these tests are failing in Windows. > > I'd like to propose that we add the Windows jobs to our PR checks if we > plan to keep testing on Windows in CI. > > Please vote or discuss. > > Thanks, > Kirk >
Re: [PROPOSAL] Add windows jobs to PR checks
+1, what was the reason for it not being included the PR before? From: Dick Cavender Sent: Thursday, June 25, 2020 9:54 AM To: dev@geode.apache.org Subject: RE: [PROPOSAL] Add windows jobs to PR checks +1 -Original Message- From: Owen Nichols Sent: Thursday, June 25, 2020 9:38 AM To: dev@geode.apache.org Subject: Re: [PROPOSAL] Add windows jobs to PR checks +1 for adding all JDK11 Windows tests to PR pipeline. On 6/25/20, 9:29 AM, "Kirk Lund" wrote: I merged some new AcceptanceTests to develop after having my PR go GREEN. But now these tests are failing in Windows. I'd like to propose that we add the Windows jobs to our PR checks if we plan to keep testing on Windows in CI. Please vote or discuss. Thanks, Kirk
RE: [PROPOSAL] Add windows jobs to PR checks
+1 -Original Message- From: Owen Nichols Sent: Thursday, June 25, 2020 9:38 AM To: dev@geode.apache.org Subject: Re: [PROPOSAL] Add windows jobs to PR checks +1 for adding all JDK11 Windows tests to PR pipeline. On 6/25/20, 9:29 AM, "Kirk Lund" wrote: I merged some new AcceptanceTests to develop after having my PR go GREEN. But now these tests are failing in Windows. I'd like to propose that we add the Windows jobs to our PR checks if we plan to keep testing on Windows in CI. Please vote or discuss. Thanks, Kirk
Re: [PROPOSAL] Add windows jobs to PR checks
+1 for adding all JDK11 Windows tests to PR pipeline. On 6/25/20, 9:29 AM, "Kirk Lund" wrote: I merged some new AcceptanceTests to develop after having my PR go GREEN. But now these tests are failing in Windows. I'd like to propose that we add the Windows jobs to our PR checks if we plan to keep testing on Windows in CI. Please vote or discuss. Thanks, Kirk
