Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for waiting for the first check to start #107

Merged
merged 2 commits into from
Apr 15, 2022

Conversation

DarkKirb
Copy link
Contributor

Hi,

I was looking into this action for use in a CD setup on NixOS and Hydra. Hydra may take a minute or two to send the first pending checks update to GitHub. By that time the action has already given up and allowed the deployment to continue.

This PR adds an additional option to this action, which will wait for all checks being completed (with at least one started)

PS: Currently the mutation testing CI step fails. I am not a PHP programmer so I am unsure what kind of additional test cases would be needed. I also haven’t gotten infection working so I cannot test locally and get more than just “Improve your test suite”

Copy link
Owner

@WyriHaximus WyriHaximus left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was looking into this action for use in a CD setup on NixOS and Hydra. Hydra may take a minute or two to send the first pending checks update to GitHub. By that time the action has already given up and allowed the deployment to continue.

This PR adds an additional option to this action, which will wait for all checks being completed (with at least one started)

That's a pretty sweet idea, thanks! (Might even make this the default in the next major version.)

PS: Currently the mutation testing CI step fails. I am not a PHP programmer so I am unsure what kind of additional test cases would be needed. I also haven’t gotten infection working so I cannot test locally and get more than just “Improve your test suite”

Will have a look at what mutation is uncovered but I have a hunch which line is causing it.

}

public function refresh(): PromiseInterface
{
return $this->combinedStatus->refresh()->then(function (Commit\CombinedStatus $status): PromiseInterface {
if ($status->totalCount() === 0) {
if ($this->waitForCheck) {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Haven't looked at it yet but I suspect the mutation test triggers this line

@WyriHaximus WyriHaximus merged commit 363ce19 into WyriHaximus:master Apr 15, 2022
WyriHaximus added a commit that referenced this pull request Apr 17, 2022
Follow up on @DarkKirb's #107, which I accidentally merged 🤐 . This ensures it
covers the new feature in the mutation tests. Plus it comes with enable some composer plugins.
@WyriHaximus WyriHaximus mentioned this pull request Apr 17, 2022
github-actions bot pushed a commit that referenced this pull request Apr 18, 2022
======

- Total issues resolved: **0**
- Total pull requests resolved: **22**
- Total contributors: **4**

Dependencies 📦,Docker 🐳
-----------------------

 - [110: Always use latest Alpine version](#110) thanks to @WyriHaximus

CI 🚧,Configuration ⚙,Docker 🐳,YAML 🍄
------------------------------------

 - [109: Use Trivy for image CVE scanning](#109) thanks to @WyriHaximus

Dependencies 📦,Enhancement ✨,JSON 👨‍💼,PHP 🐘,Tests 🧪
---------------------------------------------------

 - [108: Test wait for status](#108) thanks to @WyriHaximus

MarkDown 📝,PHP 🐘,Source 🔮,Tests 🧪,YAML 🍄
----------------------------------------

 - [107: Add support for waiting for the first check to start](#107) thanks to @DarkKirb

Dependencies 📦,JSON 👨‍💼,PHP 🐘
-----------------------------

 - [106: Ensure we check all checks](#106) thanks to @WyriHaximus

CI 🚧,Configuration ⚙,YAML 🍄
---------------------------

 - [105: Add note about how the Docker tags work](#105) thanks to @WyriHaximus

Dependencies 📦,PHP 🐘
--------------------

 - [102: Bump guzzlehttp/psr7 from 1.8.3 to 1.8.5](#102) thanks to @dependabot[bot]
 - [100: Bump monolog/monolog from 2.3.5 to 2.4.0](#100) thanks to @dependabot[bot]
 - [99: Bump wyrihaximus/async-test-utilities from 4.2.0 to 4.2.2](#99) thanks to @dependabot[bot]
 - [97: Bump wyrihaximus/psr-3-callable-throwable-logger from 2.2.0 to 2.3.0](#97) thanks to @dependabot[bot]
 - [95: Bump wyrihaximus/async-test-utilities from 4.0.8 to 4.2.0](#95) thanks to @dependabot[bot]
 - [93: Bump reactivex/rxphp from 2.0.9 to 2.0.10](#93) thanks to @dependabot[bot]
 - [92: Bump bramus/monolog-colored-line-formatter from 3.0.5 to 3.0.6](#92) thanks to @dependabot[bot]
 - [91: Bump wyrihaximus/async-test-utilities from 4.0.7 to 4.0.8](#91) thanks to @dependabot[bot]
 - [90: Bump wyrihaximus/async-test-utilities from 4.0.6 to 4.0.7](#90) thanks to @dependabot[bot]
 - [87: Bump composer/composer from 1.10.22 to 1.10.23](#87) thanks to @dependabot[bot]
 - [85: Bump lcobucci/jwt from 3.4.2 to 3.4.6](#85) thanks to @dependabot[bot]
 - [84: Bump react/stream from 1.1.1 to 1.2.0](#84) thanks to @dependabot[bot]
 - [83: Bump monolog/monolog from 2.2.0 to 2.3.3](#83) thanks to @dependabot[bot]

PHP 🐘
-----

 - [89: Switch to global event loop](#89) thanks to @WyriHaximus

Dependencies 📦,JSON 👨‍💼,NEON 🦹‍♂️,PHP 🐘,Source 🔮,Tests 🧪
--------------------------------------------------------

 - [88: Bump wyrihaximus/psr-3-callable-throwable-logger from 2.1.0 to 2.2.0](#88) thanks to @dependabot[bot]

MarkDown 📝
----------

 - [86: Clarify `ignoreActions` in README](#86) thanks to @mirka
github-actions bot pushed a commit that referenced this pull request Apr 18, 2022
======

- Total issues resolved: **0**
- Total pull requests resolved: **22**
- Total contributors: **4**

Dependencies 📦,Docker 🐳
-----------------------

 - [110: Always use latest Alpine version](#110) thanks to @WyriHaximus

CI 🚧,Configuration ⚙,Docker 🐳,YAML 🍄
------------------------------------

 - [109: Use Trivy for image CVE scanning](#109) thanks to @WyriHaximus

Dependencies 📦,Enhancement ✨,JSON 👨‍💼,PHP 🐘,Tests 🧪
---------------------------------------------------

 - [108: Test wait for status](#108) thanks to @WyriHaximus

MarkDown 📝,PHP 🐘,Source 🔮,Tests 🧪,YAML 🍄
----------------------------------------

 - [107: Add support for waiting for the first check to start](#107) thanks to @DarkKirb

Dependencies 📦,JSON 👨‍💼,PHP 🐘
-----------------------------

 - [106: Ensure we check all checks](#106) thanks to @WyriHaximus

CI 🚧,Configuration ⚙,YAML 🍄
---------------------------

 - [105: Add note about how the Docker tags work](#105) thanks to @WyriHaximus

Dependencies 📦,PHP 🐘
--------------------

 - [102: Bump guzzlehttp/psr7 from 1.8.3 to 1.8.5](#102) thanks to @dependabot[bot]
 - [100: Bump monolog/monolog from 2.3.5 to 2.4.0](#100) thanks to @dependabot[bot]
 - [99: Bump wyrihaximus/async-test-utilities from 4.2.0 to 4.2.2](#99) thanks to @dependabot[bot]
 - [97: Bump wyrihaximus/psr-3-callable-throwable-logger from 2.2.0 to 2.3.0](#97) thanks to @dependabot[bot]
 - [95: Bump wyrihaximus/async-test-utilities from 4.0.8 to 4.2.0](#95) thanks to @dependabot[bot]
 - [93: Bump reactivex/rxphp from 2.0.9 to 2.0.10](#93) thanks to @dependabot[bot]
 - [92: Bump bramus/monolog-colored-line-formatter from 3.0.5 to 3.0.6](#92) thanks to @dependabot[bot]
 - [91: Bump wyrihaximus/async-test-utilities from 4.0.7 to 4.0.8](#91) thanks to @dependabot[bot]
 - [90: Bump wyrihaximus/async-test-utilities from 4.0.6 to 4.0.7](#90) thanks to @dependabot[bot]
 - [87: Bump composer/composer from 1.10.22 to 1.10.23](#87) thanks to @dependabot[bot]
 - [85: Bump lcobucci/jwt from 3.4.2 to 3.4.6](#85) thanks to @dependabot[bot]
 - [84: Bump react/stream from 1.1.1 to 1.2.0](#84) thanks to @dependabot[bot]
 - [83: Bump monolog/monolog from 2.2.0 to 2.3.3](#83) thanks to @dependabot[bot]

PHP 🐘
-----

 - [89: Switch to global event loop](#89) thanks to @WyriHaximus

Dependencies 📦,JSON 👨‍💼,NEON 🦹‍♂️,PHP 🐘,Source 🔮,Tests 🧪
--------------------------------------------------------

 - [88: Bump wyrihaximus/psr-3-callable-throwable-logger from 2.1.0 to 2.2.0](#88) thanks to @dependabot[bot]

MarkDown 📝
----------

 - [86: Clarify `ignoreActions` in README](#86) thanks to @mirka
@WyriHaximus
Copy link
Owner

v1.5.0 is out with this PR in it, thanks again 👍.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants