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

Ensure Logger tests aren't run in parallel #1277

Conversation

robertmaynard
Copy link
Contributor

@robertmaynard robertmaynard commented May 30, 2023

Description

The logger tests all read and write to the same temporary files so we extend the tests to generate temporary directories to write the files. This will ensure that concurrent execution of the logger test variants is supported.

Due to the current limitations of rapids-cmake in 23.06 we can't use the SERIAL or RESOURCE_LOCK functions of CTest
to instead limit execution to be serial.

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 logger tests all read and write to the same temporary files
so make sure the tests are mutually exclusive.
@robertmaynard robertmaynard added bug Something isn't working 3 - Ready for review Ready for review by team ! - Hotfix Hotfix is a bug that affects the majority of users for which there is no reasonable workaround non-breaking Non-breaking change labels May 30, 2023
@robertmaynard robertmaynard requested a review from a team as a code owner May 30, 2023 22:08
@github-actions github-actions bot added CMake cpp Pertains to C++ code labels May 30, 2023
Copy link
Member

@harrism harrism left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a good quick fix. Thanks.

@harrism
Copy link
Member

harrism commented May 31, 2023

Can you fix the cmake style failure, @robertmaynard ?

@robertmaynard robertmaynard force-pushed the bug/logger_tests_mutually_exclusive branch from 40c65c6 to 8829878 Compare May 31, 2023 13:25
@robertmaynard robertmaynard requested a review from a team as a code owner May 31, 2023 13:25
@github-actions github-actions bot removed the CMake label May 31, 2023
Copy link
Contributor

@bdice bdice left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@bdice
Copy link
Contributor

bdice commented May 31, 2023

I'll request an admin-merge here.

@bdice bdice added 5 - Ready to Merge Testing and reviews complete, ready to merge and removed 3 - Ready for review Ready for review by team labels May 31, 2023
@raydouglass raydouglass merged commit c3ab060 into rapidsai:branch-23.06 May 31, 2023
40 of 41 checks passed
@robertmaynard robertmaynard deleted the bug/logger_tests_mutually_exclusive branch May 31, 2023 17:51
rapids-bot bot pushed a commit that referenced this pull request Jun 9, 2023
Corrects a bug introduced in #1277 which caused the logger tests to construct temporary directories in the build directory ( that never go deleted ).

Authors:
  - Robert Maynard (https://github.com/robertmaynard)

Approvers:
  - Mark Harris (https://github.com/harrism)
  - Vyas Ramasubramani (https://github.com/vyasr)

URL: #1289
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
5 - Ready to Merge Testing and reviews complete, ready to merge ! - Hotfix Hotfix is a bug that affects the majority of users for which there is no reasonable workaround bug Something isn't working cpp Pertains to C++ code non-breaking Non-breaking change
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

None yet

4 participants