Mailing List Archive: Github PR Actions

Github PR Actions

Sep 17, 2020, 8:56 AM

Post #1 of 12 (1166 views)

Thought I'd make this a thread instead of a discussion on a single JIRA
ticket.

Currently we have gradle precommit run on PRs for master, which is very
useful and gives people confidence in approving PRs. But precommit is
obviously not the only thing we care about before committing. It would be
great to run all the tests every time, but clearly that is too expensive.

In SOLR-14856 <https://issues.apache.org/jira/browse/SOLR-14856>, I
proposed adding a github action to build and test the solr docker image for
PRs that affected relevant parts of the repo (solr/docker, solr/bin,
solr/packaging and solr/contrib/prometheus-exporter/bin). Running the
docker tests currently takes roughly 12 minutes in the github action, which
would be costly if it ran on every PR. But when running on the small
percentage of PRs that affect those code paths, I think the benefit
outweighs the cost.

Beyond just the docker tests, I think we can leverage this ability for
other features that are limited to certain code paths. For example running
tests for contrib modules, testing solr/examples, and many of
the independent lucene modules. The SolrJ tests just ran in 3 minutes
locally for me, maybe that'd be a good candidate as well.

Anyways I'm sure there are other good candidates out there, but I just
wanted to start the discussion and hear other opinions before diving any
deeper.

Re: Github PR Actions [ In reply to ]

ichattopadhyaya at gmail

Sep 17, 2020, 3:23 PM

Post #2 of 12 (1166 views)

Permalink

> It would be great to run all the tests every time, but clearly that is
too expensive.

The reference_impl branch requires around 30 seconds to run all solr-core
tests. That's where we should all put our collective efforts.
Also, I have reservations against docker based tests blocking PRs. If I
don't have docker running on my dev machine, I wouldn't be able to make
those tests pass. This may block my ability to merge any PR whatsoever.
Why can't we have integration tests that do not rely on docker?

On Thu, Sep 17, 2020 at 9:26 PM Houston Putman <houstonputman@gmail.com>
wrote:

> Thought I'd make this a thread instead of a discussion on a single JIRA
> ticket.
>
> Currently we have gradle precommit run on PRs for master, which is very
> useful and gives people confidence in approving PRs. But precommit is
> obviously not the only thing we care about before committing. It would be
> great to run all the tests every time, but clearly that is too expensive.
>
> In SOLR-14856 <https://issues.apache.org/jira/browse/SOLR-14856>, I
> proposed adding a github action to build and test the solr docker image for
> PRs that affected relevant parts of the repo (solr/docker, solr/bin,
> solr/packaging and solr/contrib/prometheus-exporter/bin). Running the
> docker tests currently takes roughly 12 minutes in the github action, which
> would be costly if it ran on every PR. But when running on the small
> percentage of PRs that affect those code paths, I think the benefit
> outweighs the cost.
>
> Beyond just the docker tests, I think we can leverage this ability for
> other features that are limited to certain code paths. For example running
> tests for contrib modules, testing solr/examples, and many of
> the independent lucene modules. The SolrJ tests just ran in 3 minutes
> locally for me, maybe that'd be a good candidate as well.
>
> Anyways I'm sure there are other good candidates out there, but I just
> wanted to start the discussion and hear other opinions before diving any
> deeper.
>

Re: Github PR Actions [ In reply to ]

houstonputman at gmail

Sep 18, 2020, 8:54 AM

Post #3 of 12 (1163 views)

Permalink

Good point on the reference_impl branch. Eventually that's the goal, but
given there's not a timeline for that to be merged yet I think this is a
good stop-gap. It's a few minutes of work to get these PR actions written,
so I feel like there is little downside. And we can always remove them when
the reference_imp branch gets merged.

This may block my ability to merge any PR whatsoever.

The docker tests will not be run on any PRs that don't touch bin/solr,
solr/packaging or solr/docker.

There are complications around running integration tests in a
non-containerized environment as well. And if the docker image tests are
failing on the PR, wouldn't you rather know before committing? Even if you
don't want to install docker, you can call in someone else that has it to
help debug. Much like PRs that affect solr.cmd, for committers without
access to a windows machine.

On Thu, Sep 17, 2020 at 6:23 PM Ishan Chattopadhyaya <
ichattopadhyaya@gmail.com> wrote:

> > It would be great to run all the tests every time, but clearly that is
> too expensive.
>
> The reference_impl branch requires around 30 seconds to run all solr-core
> tests. That's where we should all put our collective efforts.
> Also, I have reservations against docker based tests blocking PRs. If I
> don't have docker running on my dev machine, I wouldn't be able to make
> those tests pass. This may block my ability to merge any PR whatsoever.
> Why can't we have integration tests that do not rely on docker?
>
> On Thu, Sep 17, 2020 at 9:26 PM Houston Putman <houstonputman@gmail.com>
> wrote:
>
>> Thought I'd make this a thread instead of a discussion on a single JIRA
>> ticket.
>>
>> Currently we have gradle precommit run on PRs for master, which is very
>> useful and gives people confidence in approving PRs. But precommit is
>> obviously not the only thing we care about before committing. It would be
>> great to run all the tests every time, but clearly that is too expensive.
>>
>> In SOLR-14856 <https://issues.apache.org/jira/browse/SOLR-14856>, I
>> proposed adding a github action to build and test the solr docker image for
>> PRs that affected relevant parts of the repo (solr/docker, solr/bin,
>> solr/packaging and solr/contrib/prometheus-exporter/bin). Running the
>> docker tests currently takes roughly 12 minutes in the github action, which
>> would be costly if it ran on every PR. But when running on the small
>> percentage of PRs that affect those code paths, I think the benefit
>> outweighs the cost.
>>
>> Beyond just the docker tests, I think we can leverage this ability for
>> other features that are limited to certain code paths. For example running
>> tests for contrib modules, testing solr/examples, and many of
>> the independent lucene modules. The SolrJ tests just ran in 3 minutes
>> locally for me, maybe that'd be a good candidate as well.
>>
>> Anyways I'm sure there are other good candidates out there, but I just
>> wanted to start the discussion and hear other opinions before diving any
>> deeper.
>>
>

Re: Github PR Actions [ In reply to ]

atri at apache

Sep 18, 2020, 9:05 AM

Post #4 of 12 (1163 views)

Permalink

+1 to not depending on Docker for local tests.

I do not wish to derail this thread — but re: reference branch, doesn’t it
have a bunch of tests disabled?

On Fri, 18 Sep 2020 at 03:53, Ishan Chattopadhyaya <
ichattopadhyaya@gmail.com> wrote:

> > It would be great to run all the tests every time, but clearly that is
> too expensive.
>
> The reference_impl branch requires around 30 seconds to run all solr-core
> tests. That's where we should all put our collective efforts.
> Also, I have reservations against docker based tests blocking PRs. If I
> don't have docker running on my dev machine, I wouldn't be able to make
> those tests pass. This may block my ability to merge any PR whatsoever.
> Why can't we have integration tests that do not rely on docker?
>
> On Thu, Sep 17, 2020 at 9:26 PM Houston Putman <houstonputman@gmail.com>
> wrote:
>
>> Thought I'd make this a thread instead of a discussion on a single JIRA
>> ticket.
>>
>> Currently we have gradle precommit run on PRs for master, which is very
>> useful and gives people confidence in approving PRs. But precommit is
>> obviously not the only thing we care about before committing. It would be
>> great to run all the tests every time, but clearly that is too expensive.
>>
>> In SOLR-14856 <https://issues.apache.org/jira/browse/SOLR-14856>, I
>> proposed adding a github action to build and test the solr docker image for
>> PRs that affected relevant parts of the repo (solr/docker, solr/bin,
>> solr/packaging and solr/contrib/prometheus-exporter/bin). Running the
>> docker tests currently takes roughly 12 minutes in the github action, which
>> would be costly if it ran on every PR. But when running on the small
>> percentage of PRs that affect those code paths, I think the benefit
>> outweighs the cost.
>>
>> Beyond just the docker tests, I think we can leverage this ability for
>> other features that are limited to certain code paths. For example running
>> tests for contrib modules, testing solr/examples, and many of
>> the independent lucene modules. The SolrJ tests just ran in 3 minutes
>> locally for me, maybe that'd be a good candidate as well.
>>
>> Anyways I'm sure there are other good candidates out there, but I just
>> wanted to start the discussion and hear other opinions before diving any
>> deeper.
>>
>>
>>
>
> --
Regards,

Atri
Apache Concerted

Re: Github PR Actions [ In reply to ]

tomasflobbe at gmail

Sep 18, 2020, 9:59 AM

Post #5 of 12 (1163 views)

Permalink

I think this is a good idea. In general, I'm +1 on improving PR validations
as much as possible, and as Houston says, we can always remove them later
if it's not helping. I also agree with David in his Jira comment that even
more important than this is to have the tests running on Jenkins, but I
don't see why we can't have both.

Regards,

Tomás

On Fri, Sep 18, 2020 at 9:05 AM Atri Sharma <atri@apache.org> wrote:

> +1 to not depending on Docker for local tests.
>
> I do not wish to derail this thread — but re: reference branch, doesn’t it
> have a bunch of tests disabled?
>
> On Fri, 18 Sep 2020 at 03:53, Ishan Chattopadhyaya <
> ichattopadhyaya@gmail.com> wrote:
>
>> > It would be great to run all the tests every time, but clearly that is
>> too expensive.
>>
>> The reference_impl branch requires around 30 seconds to run all solr-core
>> tests. That's where we should all put our collective efforts.
>> Also, I have reservations against docker based tests blocking PRs. If I
>> don't have docker running on my dev machine, I wouldn't be able to make
>> those tests pass. This may block my ability to merge any PR whatsoever.
>> Why can't we have integration tests that do not rely on docker?
>>
>> On Thu, Sep 17, 2020 at 9:26 PM Houston Putman <houstonputman@gmail.com>
>> wrote:
>>
>>> Thought I'd make this a thread instead of a discussion on a single JIRA
>>> ticket.
>>>
>>> Currently we have gradle precommit run on PRs for master, which is very
>>> useful and gives people confidence in approving PRs. But precommit is
>>> obviously not the only thing we care about before committing. It would be
>>> great to run all the tests every time, but clearly that is too expensive.
>>>
>>> In SOLR-14856 <https://issues.apache.org/jira/browse/SOLR-14856>, I
>>> proposed adding a github action to build and test the solr docker image for
>>> PRs that affected relevant parts of the repo (solr/docker, solr/bin,
>>> solr/packaging and solr/contrib/prometheus-exporter/bin). Running the
>>> docker tests currently takes roughly 12 minutes in the github action, which
>>> would be costly if it ran on every PR. But when running on the small
>>> percentage of PRs that affect those code paths, I think the benefit
>>> outweighs the cost.
>>>
>>> Beyond just the docker tests, I think we can leverage this ability for
>>> other features that are limited to certain code paths. For example running
>>> tests for contrib modules, testing solr/examples, and many of
>>> the independent lucene modules. The SolrJ tests just ran in 3 minutes
>>> locally for me, maybe that'd be a good candidate as well.
>>>
>>> Anyways I'm sure there are other good candidates out there, but I just
>>> wanted to start the discussion and hear other opinions before diving any
>>> deeper.
>>>
>>>
>>>
>>
>> --
> Regards,
>
> Atri
> Apache Concerted
>

Re: Github PR Actions [ In reply to ]

dsmiley at apache

Sep 18, 2020, 11:02 AM

Post #6 of 12 (1163 views)

Permalink

Yeah I'm definitely in favor of adding tests to GitHub actions. Even if
the action reports a failure, presumably we could opt to commit anyway if,
for example, we know that the test that failed is totally unrelated to the
work being committed and/or the failing test is related to some other known
issue.

~ David Smiley
Apache Lucene/Solr Search Developer
http://www.linkedin.com/in/davidwsmiley

On Fri, Sep 18, 2020 at 1:00 PM Tomás Fernández Löbbe <tomasflobbe@gmail.com>
wrote:

> I think this is a good idea. In general, I'm +1 on improving PR
> validations as much as possible, and as Houston says, we can always remove
> them later if it's not helping. I also agree with David in his Jira comment
> that even more important than this is to have the tests running on Jenkins,
> but I don't see why we can't have both.
>
> Regards,
>
> Tomás
>
> On Fri, Sep 18, 2020 at 9:05 AM Atri Sharma <atri@apache.org> wrote:
>
>> +1 to not depending on Docker for local tests.
>>
>> I do not wish to derail this thread — but re: reference branch, doesn’t
>> it have a bunch of tests disabled?
>>
>> On Fri, 18 Sep 2020 at 03:53, Ishan Chattopadhyaya <
>> ichattopadhyaya@gmail.com> wrote:
>>
>>> > It would be great to run all the tests every time, but clearly that is
>>> too expensive.
>>>
>>> The reference_impl branch requires around 30 seconds to run all
>>> solr-core tests. That's where we should all put our collective efforts.
>>> Also, I have reservations against docker based tests blocking PRs. If I
>>> don't have docker running on my dev machine, I wouldn't be able to make
>>> those tests pass. This may block my ability to merge any PR whatsoever.
>>> Why can't we have integration tests that do not rely on docker?
>>>
>>> On Thu, Sep 17, 2020 at 9:26 PM Houston Putman <houstonputman@gmail.com>
>>> wrote:
>>>
>>>> Thought I'd make this a thread instead of a discussion on a single JIRA
>>>> ticket.
>>>>
>>>> Currently we have gradle precommit run on PRs for master, which is very
>>>> useful and gives people confidence in approving PRs. But precommit is
>>>> obviously not the only thing we care about before committing. It would be
>>>> great to run all the tests every time, but clearly that is too expensive.
>>>>
>>>> In SOLR-14856 <https://issues.apache.org/jira/browse/SOLR-14856>, I
>>>> proposed adding a github action to build and test the solr docker image for
>>>> PRs that affected relevant parts of the repo (solr/docker, solr/bin,
>>>> solr/packaging and solr/contrib/prometheus-exporter/bin). Running the
>>>> docker tests currently takes roughly 12 minutes in the github action, which
>>>> would be costly if it ran on every PR. But when running on the small
>>>> percentage of PRs that affect those code paths, I think the benefit
>>>> outweighs the cost.
>>>>
>>>> Beyond just the docker tests, I think we can leverage this ability for
>>>> other features that are limited to certain code paths. For example running
>>>> tests for contrib modules, testing solr/examples, and many of
>>>> the independent lucene modules. The SolrJ tests just ran in 3 minutes
>>>> locally for me, maybe that'd be a good candidate as well.
>>>>
>>>> Anyways I'm sure there are other good candidates out there, but I just
>>>> wanted to start the discussion and hear other opinions before diving any
>>>> deeper.
>>>>
>>>>
>>>>
>>>
>>> --
>> Regards,
>>
>> Atri
>> Apache Concerted
>>
>

Re: Github PR Actions [ In reply to ]

anshum at anshumgupta

Sep 18, 2020, 11:13 AM

Post #7 of 12 (1163 views)

Permalink

I like the idea as I really feel Github actions provide a ton of value.

It doesn't have to be a blocker for all cases but for it to just run would
be great. We don't really lose anything there.

On Thu, Sep 17, 2020 at 8:56 AM Houston Putman <houstonputman@gmail.com>
wrote:

> Thought I'd make this a thread instead of a discussion on a single JIRA
> ticket.
>
> Currently we have gradle precommit run on PRs for master, which is very
> useful and gives people confidence in approving PRs. But precommit is
> obviously not the only thing we care about before committing. It would be
> great to run all the tests every time, but clearly that is too expensive.
>
> In SOLR-14856 <https://issues.apache.org/jira/browse/SOLR-14856>, I
> proposed adding a github action to build and test the solr docker image for
> PRs that affected relevant parts of the repo (solr/docker, solr/bin,
> solr/packaging and solr/contrib/prometheus-exporter/bin). Running the
> docker tests currently takes roughly 12 minutes in the github action, which
> would be costly if it ran on every PR. But when running on the small
> percentage of PRs that affect those code paths, I think the benefit
> outweighs the cost.
>
> Beyond just the docker tests, I think we can leverage this ability for
> other features that are limited to certain code paths. For example running
> tests for contrib modules, testing solr/examples, and many of
> the independent lucene modules. The SolrJ tests just ran in 3 minutes
> locally for me, maybe that'd be a good candidate as well.
>
> Anyways I'm sure there are other good candidates out there, but I just
> wanted to start the discussion and hear other opinions before diving any
> deeper.
>

--
Anshum Gupta

Re: Github PR Actions [ In reply to ]

ichattopadhyaya at gmail

Sep 18, 2020, 11:14 AM

Post #8 of 12 (1163 views)

Permalink

> The docker tests will not be run on any PRs that don't touch bin/solr,
solr/packaging or solr/docker.

Sounds good, then!

On Fri, Sep 18, 2020 at 11:44 PM Anshum Gupta <anshum@anshumgupta.net>
wrote:

> I like the idea as I really feel Github actions provide a ton of value.
>
> It doesn't have to be a blocker for all cases but for it to just run would
> be great. We don't really lose anything there.
>
> On Thu, Sep 17, 2020 at 8:56 AM Houston Putman <houstonputman@gmail.com>
> wrote:
>
>> Thought I'd make this a thread instead of a discussion on a single JIRA
>> ticket.
>>
>> Currently we have gradle precommit run on PRs for master, which is very
>> useful and gives people confidence in approving PRs. But precommit is
>> obviously not the only thing we care about before committing. It would be
>> great to run all the tests every time, but clearly that is too expensive.
>>
>> In SOLR-14856 <https://issues.apache.org/jira/browse/SOLR-14856>, I
>> proposed adding a github action to build and test the solr docker image for
>> PRs that affected relevant parts of the repo (solr/docker, solr/bin,
>> solr/packaging and solr/contrib/prometheus-exporter/bin). Running the
>> docker tests currently takes roughly 12 minutes in the github action, which
>> would be costly if it ran on every PR. But when running on the small
>> percentage of PRs that affect those code paths, I think the benefit
>> outweighs the cost.
>>
>> Beyond just the docker tests, I think we can leverage this ability for
>> other features that are limited to certain code paths. For example running
>> tests for contrib modules, testing solr/examples, and many of
>> the independent lucene modules. The SolrJ tests just ran in 3 minutes
>> locally for me, maybe that'd be a good candidate as well.
>>
>> Anyways I'm sure there are other good candidates out there, but I just
>> wanted to start the discussion and hear other opinions before diving any
>> deeper.
>>
>
>
> --
> Anshum Gupta
>

Re: Github PR Actions [ In reply to ]

atri at apache

Sep 18, 2020, 11:28 AM

Post #9 of 12 (1163 views)

Permalink

+1.

Ability to run tests in Github actions should help prevent a ton of build
breakages.

Thanks for leading this, Houston!

On Fri, 18 Sep 2020 at 21:24, Houston Putman <houstonputman@gmail.com>
wrote:

> Good point on the reference_impl branch. Eventually that's the goal, but
> given there's not a timeline for that to be merged yet I think this is a
> good stop-gap. It's a few minutes of work to get these PR actions written,
> so I feel like there is little downside. And we can always remove them when
> the reference_imp branch gets merged.
>
> This may block my ability to merge any PR whatsoever.
>
>
> The docker tests will not be run on any PRs that don't touch bin/solr,
> solr/packaging or solr/docker.
>
> There are complications around running integration tests in a
> non-containerized environment as well. And if the docker image tests are
> failing on the PR, wouldn't you rather know before committing? Even if you
> don't want to install docker, you can call in someone else that has it to
> help debug. Much like PRs that affect solr.cmd, for committers without
> access to a windows machine.
>
> On Thu, Sep 17, 2020 at 6:23 PM Ishan Chattopadhyaya <
> ichattopadhyaya@gmail.com> wrote:
>
>> > It would be great to run all the tests every time, but clearly that is
>> too expensive.
>>
>> The reference_impl branch requires around 30 seconds to run all solr-core
>> tests. That's where we should all put our collective efforts.
>> Also, I have reservations against docker based tests blocking PRs. If I
>> don't have docker running on my dev machine, I wouldn't be able to make
>> those tests pass. This may block my ability to merge any PR whatsoever.
>> Why can't we have integration tests that do not rely on docker?
>>
>> On Thu, Sep 17, 2020 at 9:26 PM Houston Putman <houstonputman@gmail.com>
>> wrote:
>>
>>> Thought I'd make this a thread instead of a discussion on a single JIRA
>>> ticket.
>>>
>>> Currently we have gradle precommit run on PRs for master, which is very
>>> useful and gives people confidence in approving PRs. But precommit is
>>> obviously not the only thing we care about before committing. It would be
>>> great to run all the tests every time, but clearly that is too expensive.
>>>
>>> In SOLR-14856 <https://issues.apache.org/jira/browse/SOLR-14856>, I
>>> proposed adding a github action to build and test the solr docker image for
>>> PRs that affected relevant parts of the repo (solr/docker, solr/bin,
>>> solr/packaging and solr/contrib/prometheus-exporter/bin). Running the
>>> docker tests currently takes roughly 12 minutes in the github action, which
>>> would be costly if it ran on every PR. But when running on the small
>>> percentage of PRs that affect those code paths, I think the benefit
>>> outweighs the cost.
>>>
>>> Beyond just the docker tests, I think we can leverage this ability for
>>> other features that are limited to certain code paths. For example running
>>> tests for contrib modules, testing solr/examples, and many of
>>> the independent lucene modules. The SolrJ tests just ran in 3 minutes
>>> locally for me, maybe that'd be a good candidate as well.
>>>
>>> Anyways I'm sure there are other good candidates out there, but I just
>>> wanted to start the discussion and hear other opinions before diving any
>>> deeper.
>>>
>>>
>>>
>>
>>
>
> --
Regards,

Atri
Apache Concerted

Re: Github PR Actions [ In reply to ]

houstonputman at gmail

Sep 18, 2020, 2:04 PM

Post #10 of 12 (1163 views)

Permalink

Thanks for the feedback everyone!

I'll get us started off with the docker and SolrJ tests. We can revisit
after a few weeks and see how it has gone.

- Houston

On Fri, Sep 18, 2020 at 2:28 PM Atri Sharma <atri@apache.org> wrote:

> +1.
>
> Ability to run tests in Github actions should help prevent a ton of build
> breakages.
>
> Thanks for leading this, Houston!
>
> On Fri, 18 Sep 2020 at 21:24, Houston Putman <houstonputman@gmail.com>
> wrote:
>
>> Good point on the reference_impl branch. Eventually that's the goal, but
>> given there's not a timeline for that to be merged yet I think this is a
>> good stop-gap. It's a few minutes of work to get these PR actions written,
>> so I feel like there is little downside. And we can always remove them when
>> the reference_imp branch gets merged.
>>
>> This may block my ability to merge any PR whatsoever.
>>
>>
>> The docker tests will not be run on any PRs that don't touch bin/solr,
>> solr/packaging or solr/docker.
>>
>> There are complications around running integration tests in a
>> non-containerized environment as well. And if the docker image tests are
>> failing on the PR, wouldn't you rather know before committing? Even if you
>> don't want to install docker, you can call in someone else that has it to
>> help debug. Much like PRs that affect solr.cmd, for committers without
>> access to a windows machine.
>>
>> On Thu, Sep 17, 2020 at 6:23 PM Ishan Chattopadhyaya <
>> ichattopadhyaya@gmail.com> wrote:
>>
>>> > It would be great to run all the tests every time, but clearly that is
>>> too expensive.
>>>
>>> The reference_impl branch requires around 30 seconds to run all
>>> solr-core tests. That's where we should all put our collective efforts.
>>> Also, I have reservations against docker based tests blocking PRs. If I
>>> don't have docker running on my dev machine, I wouldn't be able to make
>>> those tests pass. This may block my ability to merge any PR whatsoever.
>>> Why can't we have integration tests that do not rely on docker?
>>>
>>> On Thu, Sep 17, 2020 at 9:26 PM Houston Putman <houstonputman@gmail.com>
>>> wrote:
>>>
>>>> Thought I'd make this a thread instead of a discussion on a single JIRA
>>>> ticket.
>>>>
>>>> Currently we have gradle precommit run on PRs for master, which is very
>>>> useful and gives people confidence in approving PRs. But precommit is
>>>> obviously not the only thing we care about before committing. It would be
>>>> great to run all the tests every time, but clearly that is too expensive.
>>>>
>>>> In SOLR-14856 <https://issues.apache.org/jira/browse/SOLR-14856>, I
>>>> proposed adding a github action to build and test the solr docker image for
>>>> PRs that affected relevant parts of the repo (solr/docker, solr/bin,
>>>> solr/packaging and solr/contrib/prometheus-exporter/bin). Running the
>>>> docker tests currently takes roughly 12 minutes in the github action, which
>>>> would be costly if it ran on every PR. But when running on the small
>>>> percentage of PRs that affect those code paths, I think the benefit
>>>> outweighs the cost.
>>>>
>>>> Beyond just the docker tests, I think we can leverage this ability for
>>>> other features that are limited to certain code paths. For example running
>>>> tests for contrib modules, testing solr/examples, and many of
>>>> the independent lucene modules. The SolrJ tests just ran in 3 minutes
>>>> locally for me, maybe that'd be a good candidate as well.
>>>>
>>>> Anyways I'm sure there are other good candidates out there, but I just
>>>> wanted to start the discussion and hear other opinions before diving any
>>>> deeper.
>>>>
>>>>
>>>>
>>>
>>>
>>
>> --
> Regards,
>
> Atri
> Apache Concerted
>

Re: Github PR Actions [ In reply to ]

worah.gautam at gmail

Oct 9, 2020, 11:42 AM

Post #11 of 12 (1149 views)

Permalink

Hi everyone,

I was wondering why we don't run every test on every PR.
I saw the earlier comment which said it was too expensive but it would be
good if we had that feature.
As an experiment I ran all tests in a Github PR action here
<https://github.com/gautamworah96/lucene-solr/pull/2/checks?check_run_id=1223630542>
and it took ~3h 51m.

I think it is useful because sometimes we unknowingly break something in
another module and might not catch it locally because we run `./gradlew
test` only on the submodule that we've modified.
It would also provide some benefit to reviewers (more confidence in
merging?) and reduce breakages in builds.

Looking forward to your feedback!

- Gautam

On Fri, Sep 18, 2020 at 2:04 PM Houston Putman <houstonputman@gmail.com>
wrote:

> Thanks for the feedback everyone!
>
> I'll get us started off with the docker and SolrJ tests. We can revisit
> after a few weeks and see how it has gone.
>
> - Houston
>
> On Fri, Sep 18, 2020 at 2:28 PM Atri Sharma <atri@apache.org> wrote:
>
>> +1.
>>
>> Ability to run tests in Github actions should help prevent a ton of build
>> breakages.
>>
>> Thanks for leading this, Houston!
>>
>> On Fri, 18 Sep 2020 at 21:24, Houston Putman <houstonputman@gmail.com>
>> wrote:
>>
>>> Good point on the reference_impl branch. Eventually that's the goal, but
>>> given there's not a timeline for that to be merged yet I think this is a
>>> good stop-gap. It's a few minutes of work to get these PR actions written,
>>> so I feel like there is little downside. And we can always remove them when
>>> the reference_imp branch gets merged.
>>>
>>> This may block my ability to merge any PR whatsoever.
>>>
>>>
>>> The docker tests will not be run on any PRs that don't touch bin/solr,
>>> solr/packaging or solr/docker.
>>>
>>> There are complications around running integration tests in a
>>> non-containerized environment as well. And if the docker image tests are
>>> failing on the PR, wouldn't you rather know before committing? Even if you
>>> don't want to install docker, you can call in someone else that has it to
>>> help debug. Much like PRs that affect solr.cmd, for committers without
>>> access to a windows machine.
>>>
>>> On Thu, Sep 17, 2020 at 6:23 PM Ishan Chattopadhyaya <
>>> ichattopadhyaya@gmail.com> wrote:
>>>
>>>> > It would be great to run all the tests every time, but clearly that
>>>> is too expensive.
>>>>
>>>> The reference_impl branch requires around 30 seconds to run all
>>>> solr-core tests. That's where we should all put our collective efforts.
>>>> Also, I have reservations against docker based tests blocking PRs. If I
>>>> don't have docker running on my dev machine, I wouldn't be able to make
>>>> those tests pass. This may block my ability to merge any PR whatsoever.
>>>> Why can't we have integration tests that do not rely on docker?
>>>>
>>>> On Thu, Sep 17, 2020 at 9:26 PM Houston Putman <houstonputman@gmail.com>
>>>> wrote:
>>>>
>>>>> Thought I'd make this a thread instead of a discussion on a single
>>>>> JIRA ticket.
>>>>>
>>>>> Currently we have gradle precommit run on PRs for master, which is
>>>>> very useful and gives people confidence in approving PRs. But precommit is
>>>>> obviously not the only thing we care about before committing. It would be
>>>>> great to run all the tests every time, but clearly that is too expensive.
>>>>>
>>>>> In SOLR-14856 <https://issues.apache.org/jira/browse/SOLR-14856>, I
>>>>> proposed adding a github action to build and test the solr docker image for
>>>>> PRs that affected relevant parts of the repo (solr/docker, solr/bin,
>>>>> solr/packaging and solr/contrib/prometheus-exporter/bin). Running the
>>>>> docker tests currently takes roughly 12 minutes in the github action, which
>>>>> would be costly if it ran on every PR. But when running on the small
>>>>> percentage of PRs that affect those code paths, I think the benefit
>>>>> outweighs the cost.
>>>>>
>>>>> Beyond just the docker tests, I think we can leverage this ability for
>>>>> other features that are limited to certain code paths. For example running
>>>>> tests for contrib modules, testing solr/examples, and many of
>>>>> the independent lucene modules. The SolrJ tests just ran in 3 minutes
>>>>> locally for me, maybe that'd be a good candidate as well.
>>>>>
>>>>> Anyways I'm sure there are other good candidates out there, but I just
>>>>> wanted to start the discussion and hear other opinions before diving any
>>>>> deeper.
>>>>>
>>>>>
>>>>>
>>>>
>>>>
>>>
>>> --
>> Regards,
>>
>> Atri
>> Apache Concerted
>>
>

Re: Github PR Actions [ In reply to ]

erickerickson at gmail

Oct 9, 2020, 3:11 PM

Post #12 of 12 (1149 views)

Permalink

We’re kind of in limbo at this point. Mark, Ishan and Noble are pushing (along with others) to make a much faster test suite, the “reference_impl”. Once that’s done, and assuming it live up to expectations, it should then be more reasonable to run all the test for each PR. We’ll have to wait a while to see.

Best,
Erick

> On Oct 9, 2020, at 2:42 PM, Gautam Worah <worah.gautam@gmail.com> wrote:
>
> Hi everyone,
>
> I was wondering why we don't run every test on every PR.
> I saw the earlier comment which said it was too expensive but it would be good if we had that feature.
> As an experiment I ran all tests in a Github PR action here and it took ~3h 51m.
>
> I think it is useful because sometimes we unknowingly break something in another module and might not catch it locally because we run `./gradlew test` only on the submodule that we've modified.
> It would also provide some benefit to reviewers (more confidence in merging?) and reduce breakages in builds.
>
> Looking forward to your feedback!
>
> - Gautam
>
> On Fri, Sep 18, 2020 at 2:04 PM Houston Putman <houstonputman@gmail.com> wrote:
> Thanks for the feedback everyone!
>
> I'll get us started off with the docker and SolrJ tests. We can revisit after a few weeks and see how it has gone.
>
> - Houston
>
> On Fri, Sep 18, 2020 at 2:28 PM Atri Sharma <atri@apache.org> wrote:
> +1.
>
> Ability to run tests in Github actions should help prevent a ton of build breakages.
>
> Thanks for leading this, Houston!
>
> On Fri, 18 Sep 2020 at 21:24, Houston Putman <houstonputman@gmail.com> wrote:
> Good point on the reference_impl branch. Eventually that's the goal, but given there's not a timeline for that to be merged yet I think this is a good stop-gap. It's a few minutes of work to get these PR actions written, so I feel like there is little downside. And we can always remove them when the reference_imp branch gets merged.
>
> This may block my ability to merge any PR whatsoever.
>
> The docker tests will not be run on any PRs that don't touch bin/solr, solr/packaging or solr/docker.
>
> There are complications around running integration tests in a non-containerized environment as well. And if the docker image tests are failing on the PR, wouldn't you rather know before committing? Even if you don't want to install docker, you can call in someone else that has it to help debug. Much like PRs that affect solr.cmd, for committers without access to a windows machine.
>
> On Thu, Sep 17, 2020 at 6:23 PM Ishan Chattopadhyaya <ichattopadhyaya@gmail.com> wrote:
> > It would be great to run all the tests every time, but clearly that is too expensive.
>
> The reference_impl branch requires around 30 seconds to run all solr-core tests. That's where we should all put our collective efforts.
> Also, I have reservations against docker based tests blocking PRs. If I don't have docker running on my dev machine, I wouldn't be able to make those tests pass. This may block my ability to merge any PR whatsoever.
> Why can't we have integration tests that do not rely on docker?
>
> On Thu, Sep 17, 2020 at 9:26 PM Houston Putman <houstonputman@gmail.com> wrote:
> Thought I'd make this a thread instead of a discussion on a single JIRA ticket.
>
> Currently we have gradle precommit run on PRs for master, which is very useful and gives people confidence in approving PRs. But precommit is obviously not the only thing we care about before committing. It would be great to run all the tests every time, but clearly that is too expensive.
>
> In SOLR-14856, I proposed adding a github action to build and test the solr docker image for PRs that affected relevant parts of the repo (solr/docker, solr/bin, solr/packaging and solr/contrib/prometheus-exporter/bin). Running the docker tests currently takes roughly 12 minutes in the github action, which would be costly if it ran on every PR. But when running on the small percentage of PRs that affect those code paths, I think the benefit outweighs the cost.
>
> Beyond just the docker tests, I think we can leverage this ability for other features that are limited to certain code paths. For example running tests for contrib modules, testing solr/examples, and many of the independent lucene modules. The SolrJ tests just ran in 3 minutes locally for me, maybe that'd be a good candidate as well.
>
> Anyways I'm sure there are other good candidates out there, but I just wanted to start the discussion and hear other opinions before diving any deeper.
>
>
>
>
>
>
> --
> Regards,
>
> Atri
> Apache Concerted

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: dev-help@lucene.apache.org