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

[ci] test 32-bit R in CI #3588

Merged
merged 3 commits into from
Nov 23, 2020
Merged

Conversation

jameslamb
Copy link
Collaborator

@jameslamb jameslamb commented Nov 22, 2020

Based on #3585 (comment). We were recently surprised by some test failures on 32-bit Windows checks for CRAN.

I think this is because I thought we were running tests with a 32-bit version of R on Windows (#3311 ) but maybe we never actually were.

I think that the root cause is that the test setup for our R environment doesn't install i386 components for R-win.exe, as documented in https://cran.r-project.org/bin/windows/base/rw-FAQ.html#Can-I-customize-the-installation_003f.

As of this PR, 32-bit R will be installed in GitHub Actions jobs and the R package will be tested against both architectures (x86 and i386).

Questions for Reviewers

In #3585 , we ran into issues only on CRAN's R 3.6 + Windows (32-bit) job. Today, the only tests of the CRAN package are on R 4.0. That's a gap between this project's CI and CRAN, which means we could run into this situation again in future CRAN submission. In this PR I'd like to also add an R 3.6 + Windows + build_type: cran job.

GitHub Actions gives you 20 concurrent job runs, and right now we have 17 jobs that run on every PR.

How do you feel about adding this additional job?

          - os: windows-latest
            task: r-package
            compiler: MINGW
            toolchain: MINGW
            r_version: 3.6
            build_type: cran

NOTE: this PR's tests won't pass until #3586 is merged. I wanted to keep them separate so this CI work doesn't slow down the process of releasing a fix for CRAN.

@jameslamb
Copy link
Collaborator Author

I think this worked! Now the behavior in CI matches what we're seeing on CRAN...the R3.6 + cran build is failing with the exact same issues described in #3585

https://github.com/microsoft/LightGBM/pull/3588/checks?check_run_id=1437170140

  == testthat results  ===========================================================

  FAILURE (test_learning_to_rank.R:52:9): learning-to-rank with lgb.train() works as expected

  FAILURE (test_learning_to_rank.R:53:9): learning-to-rank with lgb.train() works as expected

  FAILURE (test_learning_to_rank.R:130:5): learning-to-rank with lgb.cv() works as expected

  FAILURE (test_learning_to_rank.R:136:5): learning-to-rank with lgb.cv() works as expected

And also a new compiler warning we haven't noticed before

./include/LightGBM/utils/common.h:47:0: warning: "_mm_free" redefined
 #define _mm_free(a) free(a)
 ^

./include/LightGBM/utils/common.h:46:0: warning: "_mm_malloc" redefined
 #define _mm_malloc(a, b) malloc(a)
 ^

@jameslamb jameslamb changed the title [ci] test 32-bit R in CI WIP: [ci] test 32-bit R in CI Nov 22, 2020
@StrikerRUS
Copy link
Collaborator

How do you feel about adding this additional job?

Totally fine!

@jameslamb jameslamb changed the title WIP: [ci] test 32-bit R in CI [ci] test 32-bit R in CI Nov 23, 2020
@jameslamb
Copy link
Collaborator Author

@StrikerRUS I just moved the changes to the tests from #3586 over to this PR. Since it seems like #3586 might be delayed for an unknown amount of time waiting for CRAN to reply, I think we should merge this PR with the updated tests + the new CI job sooner.

I want this new CI job to be sure that new bugfixes and features (for example, #3405) don't break the 32-bit CRAN checks.

@jameslamb
Copy link
Collaborator Author

checked the logs one more time and I can see the tests are now running for both i386 and x64 architectures

image

@github-actions
Copy link

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.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 24, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants