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

upkeep: bump r2dii version in helper-skip_if_r2dii_data_outdated.R #476

Merged
merged 2 commits into from
Apr 25, 2024

Conversation

jdhoffa
Copy link
Member

@jdhoffa jdhoffa commented Apr 25, 2024

The (brittle) snapshot tests are currently failing when run against the CRAN published r2dii.data v0.5.0. They only pass against the dev version of the package.

This PR bumps the function that skips snapshot tests with old r2dii.data.

Another indication that we should prioritize working on #392

Relates to RMI-PACTA/r2dii.data#374
Relates to #473

The (brittle) snapshot tests are currently failing when run against the CRAN published `r2dii.data v0.5.0`. They only pass against the dev version of the package.

This PR bumps the function that skips snapshot tests with old `r2dii.data`. 

Another indication that we should prioritize working on #392

Relates to RMI-PACTA/r2dii.data#374
Relates to #473
@jdhoffa jdhoffa added the upkeep maintenance, infrastructure, and similar label Apr 25, 2024
@jdhoffa jdhoffa requested a review from jacobvjk as a code owner April 25, 2024 09:50
jacobvjk
jacobvjk previously approved these changes Apr 25, 2024
@jdhoffa
Copy link
Member Author

jdhoffa commented Apr 25, 2024

Hmm ok that didn't seem to work actually.
Let me see what is going on

@jdhoffa
Copy link
Member Author

jdhoffa commented Apr 25, 2024

@jacobvjk it turns out the tests that were failing were those that tests the expected names of the output of match_name.

Since we have changed the column names in the input data, the expected names of the output will be different between 0.5.0 and 0.5.0.9000 (and beyond).

There are two options:

  • Skip the tests unless the r2dii.data version is sufficiently high
    OR
  • Soften the test to ensure that the output CONTAINS the expected names (rather than strictly expecting the EXACT names)

of the two, I prefer the former option, since we have a stronger test of match_name, and all of this is dev-facing anyway.

@jdhoffa jdhoffa requested a review from jacobvjk April 25, 2024 10:00
@jacobvjk
Copy link
Member

@jacobvjk it turns out the tests that were failing were those that tests the expected names of the output of match_name.

Since we have changed the column names in the input data, the expected names of the output will be different between 0.5.0 and 0.5.0.9000 (and beyond).

There are two options:

  • Skip the tests unless the r2dii.data version is sufficiently high
    OR
  • Soften the test to ensure that the output CONTAINS the expected names (rather than strictly expecting the EXACT names)

of the two, I prefer the former option, since we have a stronger test of match_name, and all of this is dev-facing anyway.

agree with this and definitely better to work on removing the skips by removing the test dependency on r2dii.data soon

@jdhoffa jdhoffa merged commit cef463a into main Apr 25, 2024
22 checks passed
@jdhoffa jdhoffa deleted the jdhoffa-patch-1 branch April 25, 2024 10:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
upkeep maintenance, infrastructure, and similar
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants