[R-package] [ci] Make Windows CI fail if R CMD check fails #3435

jameslamb · 2020-10-04T05:57:57Z

In #3405 (comment), I noticed that if R CMD check raises an ERROR on Windows, LightGBM's CI doesn't fail. Maybe that is a result of the way we have to intercept stderr on GitHub Actions Windows environments (

LightGBM/.ci/test_r_package_windows.ps1

Lines 15 to 28 in 0fc4c5c

    
           # External utilities like R.exe / Rscript.exe writing to stderr (even for harmless 
        
           # status information) can cause failures in GitHub Actions PowerShell jobs. 
        
           # See https://github.saobby.my.eu.orgmunity/t/powershell-steps-fail-nondeterministically/115496 
        
           # 
        
           # Using standard PowerShell redirection does not work to avoid these errors. 
        
           # This function uses R's built-in redirection mechanism, sink(). Any place where 
        
           # this function is used is a command that writes harmless messages to stderr 
        
           function Run-R-Code-Redirect-Stderr { 
        
             param( 
        
               [string]$rcode 
        
             ) 
        
             $decorated_code = "out_file <- file(tempfile(), open = 'wt'); sink(out_file, type = 'message'); $rcode; sink()" 
        
             Rscript --vanilla -e $decorated_code 
        
           }

).

This PR proposes a fix, to be sure that CI fails on changes that break the R package.

jameslamb · 2020-10-04T06:00:19Z

R-package/R/aliases.R

@@ -6,6 +6,7 @@
 #               R sources files during installation).
 # [return] A named list, where each key is a parameter relevant to lgb.DataSet and each value is a character
 #          vector of corresponding aliases.
+,,,,


I'm starting by intentionally pushing a change that raises an ERROR in R CMD check, so we can see that the Windows tests pass.

even with this syntax error, all of the Windows tests passed!

https://github.com/microsoft/LightGBM/pull/3435/checks?check_run_id=1204672524

After adding f916575, the MINGW tests fail as expected

https://github.com/microsoft/LightGBM/pull/3435/checks?check_run_id=1204732555

But the MSVC ones do not! Another issue with how errors are caught by Run-R-Code-Redirect-Stderr, maybe.

after adding e191673, all Windows checks now fail as expected

https://github.com/microsoft/LightGBM/pull/3435/checks?check_run_id=1204756009

StrikerRUS

LGTM! Looks like we cannot rely on Run-R-Code-Redirect-Stderr...

StrikerRUS · 2020-10-04T15:43:29Z

.ci/test_r_package_windows.ps1

@@ -181,6 +185,11 @@ if ($env:COMPILER -ne "MSVC") {
  Get-Content -Path "$INSTALL_LOG_FILE_NAME"
  Write-Output "----- end of build and install logs -----"
  Check-Output $install_succeeded
+  # some errors are not raised above, but can be found in the logs
+  if (Get-Content "$INSTALL_LOG_FILE_NAME" | Select-String -Pattern "ERROR" -Quiet) {
+      echo "ERRORs have been found installing lightgbm"


Oops! Seems that during VS trials and errors (we use them to find any supported VS version) we trigger this condition:

Trying 'Visual Studio 16 2019' -- Configuring incomplete, errors occurred!

I always forget that Select-String is case-insensitive by default 🙃

LGTM! Looks like we cannot rely on Run-R-Code-Redirect-Stderr...

Oh wait, I see why! I might be able to fix this by fixing Run-R-Code-Redirect-Stderr.

My next few commits here will be to test...I'll comment when it's ready for another review

ok sadly my experiment failed. I thought I just needed to add a return or Exit in Run-R-Code-Redirect-Stderr but that didn't work. All of the ways we have to catch errors makes it really complicated :/

jameslamb · 2020-10-05T01:02:15Z

LGTM! Looks like we cannot rely on Run-R-Code-Redirect-Stderr...

Oh wait, I see why! I might be able to fix this by fixing Run-R-Code-Redirect-Stderr.

My next few commits here will be to test...I'll comment when it's ready for another review

StrikerRUS

Thanks for the neat solution!

github-actions · 2023-08-24T04:51:56Z

This pull request has been automatically locked since there has not been any recent activity since it was closed. To start a new related discussion, open a new issue at https://github.com/microsoft/LightGBM/issues including a reference to this.

jameslamb added the fix label Oct 4, 2020

jameslamb requested a review from StrikerRUS October 4, 2020 05:57

jameslamb requested a review from Laurae2 as a code owner October 4, 2020 05:57

jameslamb force-pushed the fix/r-windows-ci branch from 24f24d7 to aad70cc Compare October 4, 2020 05:58

[R-package] [ci] Make Windows CI fail if R CMD check fails

6931691

jameslamb force-pushed the fix/r-windows-ci branch from aad70cc to 6931691 Compare October 4, 2020 05:59

jameslamb commented Oct 4, 2020

View reviewed changes

jameslamb added 3 commits October 4, 2020 01:32

restore fix

f916575

catch more errors

e191673

remove testing

9d719d7

StrikerRUS approved these changes Oct 4, 2020

View reviewed changes

StrikerRUS requested changes Oct 4, 2020

View reviewed changes

Merge branch 'master' into fix/r-windows-ci

e0ba9d0

fix processx calls

216c31a

jameslamb force-pushed the fix/r-windows-ci branch from 10bba36 to 216c31a Compare October 5, 2020 01:38

jameslamb added 6 commits October 4, 2020 20:51

try to get status code

1bc3dec

more logs

17f3140

trying something

56c9ff1

stuff

1a19cdc

previous changes

d4219fa

uncomment

7266e7d

AlbertoEAF mentioned this pull request Oct 6, 2020

[R-package] changes to support read/write performance in CMake builds feedzai/LightGBM#4

Closed

StrikerRUS approved these changes Oct 6, 2020

View reviewed changes

StrikerRUS merged commit 967a781 into microsoft:master Oct 6, 2020

jameslamb deleted the fix/r-windows-ci branch October 11, 2020 04:36

github-actions bot locked as resolved and limited conversation to collaborators Aug 24, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[R-package] [ci] Make Windows CI fail if R CMD check fails #3435

[R-package] [ci] Make Windows CI fail if R CMD check fails #3435

jameslamb commented Oct 4, 2020

jameslamb Oct 4, 2020

jameslamb Oct 4, 2020

jameslamb Oct 4, 2020

jameslamb Oct 4, 2020

StrikerRUS left a comment

StrikerRUS Oct 4, 2020

jameslamb Oct 5, 2020

jameslamb Oct 6, 2020

jameslamb Oct 6, 2020

jameslamb commented Oct 5, 2020

StrikerRUS left a comment

github-actions bot commented Aug 24, 2023

	# External utilities like R.exe / Rscript.exe writing to stderr (even for harmless
	# status information) can cause failures in GitHub Actions PowerShell jobs.
	# See https://github.saobby.my.eu.orgmunity/t/powershell-steps-fail-nondeterministically/115496
	#
	# Using standard PowerShell redirection does not work to avoid these errors.
	# This function uses R's built-in redirection mechanism, sink(). Any place where
	# this function is used is a command that writes harmless messages to stderr
	function Run-R-Code-Redirect-Stderr {
	param(
	[string]$rcode
	)
	$decorated_code = "out_file <- file(tempfile(), open = 'wt'); sink(out_file, type = 'message'); $rcode; sink()"
	Rscript --vanilla -e $decorated_code
	}

[R-package] [ci] Make Windows CI fail if R CMD check fails #3435

[R-package] [ci] Make Windows CI fail if R CMD check fails #3435

Conversation

jameslamb commented Oct 4, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

StrikerRUS left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jameslamb commented Oct 5, 2020

StrikerRUS left a comment

Choose a reason for hiding this comment

github-actions bot commented Aug 24, 2023