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

limit pinning tests to CPM-downloaded projects #599

Merged
merged 2 commits into from
May 1, 2024

Conversation

jameslamb
Copy link
Member

Description

Modifies pinning tests from #530:

  • to only test projects that were downloaded by CPM (e.g. ignoring the fmt that might already exist in the build environment)
  • to echo out pinned_versions.json and versions.json in logs from failed tests, to make debugging faster

Notes for Reviewers

#592 proposes some testing changes that aren't specific to the goals of that PR.

Since that PR might be stuck for a bit (rapidsai/build-planning#56 (comment)), this proposes pulling those out into a separate PR:

How I tested this

Pushed a commit with the new test error message content changes but keeping fmt in the failing tests, to confirm that the expected tests failed.

got the expected outputs (click me)
The following tests FAILED:
	698 - cpm_generate_pins-nested-makefile (Failed)
	700 - cpm_generate_pins-nested-ninja (Failed)
	702 - cpm_generate_pins-nested-ninja_multi-config (Failed)
	722 - cpm_generate_pins-simple-makefile (Failed)
	724 - cpm_generate_pins-simple-ninja (Failed)
	726 - cpm_generate_pins-simple-ninja_multi-config (Failed)

And they failed in the expected way more informative logs!

CMake Error at CMakeLists.txt:51 (message):
  pinned fmt tag (10.2.1) should differ compared to baseline 10.2.1

  pinned_versions.json:

  {

    "always_download" : true,
    "git_shallow" : false,
    "git_tag" : "${version}",
    "git_url" : "https://github.com/fmtlib/fmt.git",
    "version" : "10.2.1"

  }

  versions.json:

  {

    "git_tag" : "${version}",
    "git_url" : "https://github.com/fmtlib/fmt.git",
    "version" : "10.2.1"

  }

(build link)

Checklist

  • I am familiar with the Contributing Guidelines.
  • New or existing tests cover these changes.
  • The documentation is up to date with these changes.
  • The cmake-format.json is up to date with these changes.
  • I have added new files under rapids-cmake/
    • I have added include guards (include_guard(GLOBAL))
    • I have added the associated docs/ rst file and update the api.rst

@jameslamb jameslamb added improvement Improves an existing functionality non-breaking Introduces a non-breaking change labels May 1, 2024
@jameslamb jameslamb requested a review from a team as a code owner May 1, 2024 18:48
@robertmaynard
Copy link
Contributor

/merge

@rapids-bot rapids-bot bot merged commit bc1c006 into rapidsai:branch-24.06 May 1, 2024
15 checks passed
@jameslamb jameslamb deleted the testing-pinned-versions branch May 1, 2024 20:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
improvement Improves an existing functionality non-breaking Introduces a non-breaking change
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants