Address extendr-api CI failures #380
Conversation
|
Okay. I guess we should make an extendr patch release as soon as possible.
And rextendr clearly has some snapshot tests here that needs a cleanup.
…On Thu, Sep 5, 2024, 06:15 Josiah Parry ***@***.***> wrote:
@JosiahParry <https://github.com/JosiahParry> requested your review on:
#380 <#380> Address extendr-api
CI failures.
—
Reply to this email directly, view it on GitHub
<#380 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAIDVSAR4JQHPLTJL2KVTCLZU7LGBAVCNFSM6AAAAABNVTJECKVHI2DSMVQWIX3LMV45UABCJFZXG5LFIV3GK3TUJZXXI2LGNFRWC5DJN5XDWMJUGE2DAMRZHA2DGOI>
.
You are receiving this because your review was requested.Message ID:
***@***.***>
|
|
Why do you change defaults in {rextendr} though? If you need {rextendr} to test against main head, just configure tests so. |
I'm not going to accept these changes and merge them, as I agree with you. But we are in a pinch, and integration test with rextendr is just causing the extendr maintenance to be a nightmare. Atleast this PR shows that the integration test should pass.. I am also noticing @Ilia-Kosenkov that there are some snapshot tests in I suggest that this PR is closed, but then it would be nice if @Ilia-Kosenkov can make a way to integrate test rextendr on extendr in a reasonable way, instead of this interlocking. I also would recommend that we remove rextendr integration testing on extendr for now. It seems to me that the burden of maintaining the entire extendr ecosystem falls on people making PRs to extendr-api crate. Which is unreasonable. These are my thoughts atleast. Good job everyone. |
|
Ok, sounds fair. You can start by disabling rextendr tests in extendr for the time being. Feel free to mention me in any issues related to testing there, I might be able to help. |
|
I've takena look again. I think there are some tests, that test extendr on crates.io in rextendr. These should all be made, so they are disabled, or atleast follow the "default" version, if they are ran in extendr-ci. We have already gone through this, but I think some things fell through the cracks. |
This PR addresses the failing CI tests in extendr/extendr#842.
It does so by using the current head of the git repository for CI instead of the currently released version on crates.io.
This also updates snapshots to pass the tests.
This should let us pass CI on extendr-api and prove that we can also patch extendr-api to keep up with the changes in libR-sys due to non-API.
The question is: do we want to keep rextendr testing main? Or revert the change after being merged?