Skip to content

Conversation

@kirklandsign
Copy link
Contributor

@kirklandsign kirklandsign commented Nov 23, 2024

Summary

We can build the test while building the lib, just in one pass.

After this, developers can run tests while they develop. Both can be built at the same time. For developers, when they write their code, it's easy for them to test out. Just add -DEXECUTORCH_BUILD_TESTS=ON to their cmake config. The test target will be built automatically in cmake-out and they can run it.

NOTE: examples/models/llama/tokenizer/test and extension/llm/tokenizer/test can't use this right now. They don't have a good triggering path, and can't be triggered from root either. Use the old trigger path for them.

Test plan

Method 1: script for all tests

sh test/run_oss_cpp_tests.sh

Method 2: go to cmake-out and test

cd cmake-out
ctest

Method 3: test specific target

For example, we want to test kernel_resolution_test in runtime/core

cmake -DEXECUTORCH_BUILD_TESTS=ON . -Bcmake-out
cmake --build cmake-out
cd cmake-out
ctest -R kernel_resolution_test

@pytorch-bot
Copy link

pytorch-bot bot commented Nov 23, 2024

🔗 Helpful Links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/executorch/7043

Note: Links to docs will display an error until the docs builds have been completed.

❌ 1 New Failure

As of commit 0dcd11c with merge base 047fd37 (image):

NEW FAILURE - The following job has failed:

  • pull / unittest-arm / linux-job (gh)
    RuntimeError: Command docker exec -t e71a6b20d9e88da59e1240fd9a45348febcf1131be56e635b81d8addde1a19d9 /exec failed with exit code 1

This comment was automatically generated by Dr. CI and updates every 15 minutes.

@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Nov 23, 2024
@kirklandsign kirklandsign added the release notes: misc Miscellaneous label Nov 23, 2024
@kirklandsign kirklandsign force-pushed the simplify-test branch 2 times, most recently from 4173529 to 2b57c6c Compare November 25, 2024 23:41
CMakeLists.txt Outdated
Comment on lines 861 to 879
if(BUILD_TESTING)
# This contains the list of tests which are always built
add_subdirectory(extension/evalue_util/test)
add_subdirectory(extension/kernel_util/test)
add_subdirectory(extension/memory_allocator/test)
add_subdirectory(extension/parallel/test)
add_subdirectory(extension/pytree/test)
add_subdirectory(kernels/portable/test)
add_subdirectory(kernels/portable/cpu/util/test)
add_subdirectory(kernels/prim_ops/test)
add_subdirectory(kernels/test)
add_subdirectory(runtime/core/exec_aten/testing_util/test)
add_subdirectory(runtime/core/exec_aten/util/test)
add_subdirectory(runtime/core/portable_type/test)
add_subdirectory(runtime/core/test)
add_subdirectory(runtime/executor/test)
add_subdirectory(runtime/kernel/test)
add_subdirectory(runtime/platform/test)
add_subdirectory(test/utils)
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we move these into test.cmake


# Add googletest if any test targets should be built
if(EXECUTORCH_BUILD_GTESTS)
if(BUILD_TESTING)
Copy link
Contributor

Choose a reason for hiding this comment

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

Why not EXECUTORCH_BUILD_TESTS?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

BUILD_TESTING is from cmake system to recognize whether test config is on or not. It will be enabled when we include ctest (see https://github.com/pytorch/executorch/pull/7043/files#diff-1e7de1ae2d059d21e1dd75d5812d5a34b0222cef273b7c3a2af62eb747f9d20aR318-R320)

@facebook-github-bot
Copy link
Contributor

@kirklandsign has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

@facebook-github-bot
Copy link
Contributor

@kirklandsign has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

@facebook-github-bot
Copy link
Contributor

@kirklandsign has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

@facebook-github-bot
Copy link
Contributor

@kirklandsign has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

@facebook-github-bot
Copy link
Contributor

@kirklandsign has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

@facebook-github-bot
Copy link
Contributor

@kirklandsign has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

@facebook-github-bot facebook-github-bot merged commit cd306d3 into main Dec 5, 2024
48 of 49 checks passed
@facebook-github-bot facebook-github-bot deleted the simplify-test branch December 5, 2024 07:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. release notes: misc Miscellaneous

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants