Skip to content

Conversation

@KFilipek
Copy link
Contributor

Description

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
  • All API changes are reflected in docs and def/map files, and are tested

@KFilipek KFilipek requested a review from a team as a code owner February 28, 2025 12:53
@KFilipek KFilipek self-assigned this Feb 28, 2025
@KFilipek KFilipek force-pushed the task-unify_test_name branch from 71a24b2 to 510517e Compare February 28, 2025 13:26
@KFilipek KFilipek force-pushed the task-unify_test_name branch from 510517e to 88d5156 Compare February 28, 2025 13:48
@lplewa
Copy link
Contributor

lplewa commented Feb 28, 2025

If we are renaming test names, can we rename them in that way that we use only '-' or '_', not both. I find it really annoying when typing name of the test.

@KFilipek
Copy link
Contributor Author

If we are renaming test names, can we rename them in that way that we use only '-' or '_', not both. I find it really annoying when typing name of the test.

According to GTest documentation each test should be named using camel-case - it should solve each problem.

@KFilipek KFilipek force-pushed the task-unify_test_name branch 3 times, most recently from 8548b6d to 625139c Compare March 3, 2025 14:58
@bratpiorka bratpiorka marked this pull request as draft March 4, 2025 09:35
@KFilipek KFilipek force-pushed the task-unify_test_name branch 2 times, most recently from 4669655 to 2e7c6f4 Compare March 4, 2025 15:10
@KFilipek KFilipek marked this pull request as ready for review March 4, 2025 17:28
@KFilipek KFilipek requested review from a team and lplewa March 4, 2025 17:29
@KFilipek KFilipek force-pushed the task-unify_test_name branch from 2e7c6f4 to 51ba72a Compare March 5, 2025 09:07
@PatKamin
Copy link
Contributor

PatKamin commented Mar 5, 2025

So with this change we would have umf-test_name tests and umf_example_test_name examples binaries. Do you plan to rename examples in the same way in another PR?

Other, non-blocking, comment - the naming convention of umf tests is just for easy grepping of tests binaries as they are not installed with umf. As they are not-installed-binaries I would prefer a more convenient naming of tests like test-test_name.

@lukaszstolarczuk
Copy link
Contributor

I vote for combining both - Patryk's and Lukasz's proposals: test_<test_bin_name> (fixing test_bin_name's is totally different story)

@KFilipek KFilipek force-pushed the task-unify_test_name branch from 51ba72a to 9d797c5 Compare March 5, 2025 13:36
@KFilipek KFilipek changed the title Make umf-* prefix for tests instead of umf_test-* Make umf-* prefix for tests instead of test_* Mar 5, 2025
@KFilipek KFilipek force-pushed the task-unify_test_name branch from 9d797c5 to cb94612 Compare March 5, 2025 13:53
@lukaszstolarczuk lukaszstolarczuk merged commit 087b23e into oneapi-src:main Mar 5, 2025
223 of 230 checks passed
@lukaszstolarczuk lukaszstolarczuk changed the title Make umf-* prefix for tests instead of test_* Change umf_test-* tests' prefix to test_* Mar 5, 2025
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.

5 participants