Skip to content

Disable proxy lib tests on Windows Debug #401

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

Merged

Conversation

PatKamin
Copy link
Contributor

@PatKamin PatKamin commented Apr 2, 2024

Description

Fixes: #399

This PR disables the two tests which use proxy lib when ctest is run for a Debug config on Windows:

ctest -C Debug

Checklist

  • Code compiles without errors locally
  • All tests pass locally
  • CI workflows execute properly
  • CI workflows, not executed per PR (e.g. Nightly), execute properly
  • New tests added, especially if they will fail without my changes
  • Added/extended example(s) to cover this functionality
  • Extended the README/documentation
  • All newly added source files have a license
  • All newly added source files are referenced in CMake files
  • Logger (with debug/info/... messages) is used

@PatKamin PatKamin requested a review from a team as a code owner April 2, 2024 18:43
Copy link
Contributor

@ldorau ldorau left a comment

Choose a reason for hiding this comment

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

Now you have to add the -C option to all ctest calls, even on Linux! Because this

$ ctest

will NOT run the proxy library tests, only that:

$ ctest -C Release

will do.

ldorau

This comment was marked as resolved.

@PatKamin PatKamin force-pushed the disable-proxy-lib-tests-win-debug branch 2 times, most recently from 3a7a016 to 124b5ef Compare April 3, 2024 06:59
@bratpiorka
Copy link
Contributor

What if a user wants to use a non-standard (self made up) build type?

I think we shouldn't bother with this right now. I hope we'll fix the problems with debug proxy lib in the future and then we could remove this code.

@ldorau
Copy link
Contributor

ldorau commented Apr 3, 2024

What if a user wants to use a non-standard (self made up) build type?

I think we shouldn't bother with this right now. I hope we'll fix the problems with debug proxy lib in the future and then we could remove this code.

OK

@bratpiorka
Copy link
Contributor

Now you have to add the -C option to all ctest calls, even on Linux! Because this

$ ctest

will NOT run the proxy library tests, only that:

$ ctest -C Release

will do.

but I see the CI pass?

@PatKamin
Copy link
Contributor Author

PatKamin commented Apr 3, 2024

Now you have to add the -C option to all ctest calls, even on Linux! Because this

$ ctest

will NOT run the proxy library tests, only that:

$ ctest -C Release

will do.

but I see the CI pass?

Yes, in CI we explicitly use the -C parameter.

@PatKamin
Copy link
Contributor Author

PatKamin commented Apr 3, 2024

Now you have to add the -C option to all ctest calls, even on Linux! Because this

$ ctest

will NOT run the proxy library tests, only that:

$ ctest -C Release

will do.

Now the behavior is the same regardless of the platform, one have to add the -C option.

ldorau

This comment was marked as resolved.

@ldorau
Copy link
Contributor

ldorau commented Apr 3, 2024

The proxy library tests are not run on Linux now!:
After adding:
ldorau@684d635
all tests passed:
https://github.com/ldorau/unified-memory-framework/actions/runs/8534974992

@ldorau
Copy link
Contributor

ldorau commented Apr 3, 2024

Now you have to add the -C option to all ctest calls, even on Linux! Because this

$ ctest

will NOT run the proxy library tests, only that:

$ ctest -C Release

will do.

but I see the CI pass?

Yes, in CI we explicitly use the -C parameter.

But not in Linux jobs! - it has to be added there.

@PatKamin PatKamin force-pushed the disable-proxy-lib-tests-win-debug branch from 124b5ef to b263130 Compare April 3, 2024 08:33
@PatKamin
Copy link
Contributor Author

PatKamin commented Apr 3, 2024

Now you have to add the -C option to all ctest calls, even on Linux! Because this

$ ctest

will NOT run the proxy library tests, only that:

$ ctest -C Release

will do.

but I see the CI pass?

Yes, in CI we explicitly use the -C parameter.

But not in Linux jobs! - it has to be added there.

Thanks, I hadn't noticed that. I've removed the list of configs from the linux builds so there should be no impact of this change on linux tests now.

@PatKamin PatKamin force-pushed the disable-proxy-lib-tests-win-debug branch from b263130 to 92fa419 Compare April 3, 2024 08:52
@lukaszstolarczuk lukaszstolarczuk merged commit 31d8be1 into oneapi-src:main Apr 3, 2024
@PatKamin PatKamin deleted the disable-proxy-lib-tests-win-debug branch December 31, 2024 13:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

The proxy library tests are built in the Debug build on Windows in case of the shared version of the UMF library.
4 participants