#14690: Reorganize gtests under tests/tt_metal/tt_metal
#14691
Conversation
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
tests/tt_metal/tt_metal/unit_tests_common/dprint/test_invalid_print_core.cpp
Show resolved
Hide resolved
| @@ -1,7 +1,7 @@ | |||
| // SPDX-FileCopyrightText: © 2023 Tenstorrent Inc. | |||
| // | |||
| // SPDX-License-Identifier: Apache-2.0 | |||
| #include "tests/tt_metal/tt_metal/unit_tests_common/common/common_fixture.hpp" | |||
| #include "dispatch_fixture.hpp" | |||
There was a problem hiding this comment.
dispatch_fixture.hpp file not found
| @@ -2,7 +2,7 @@ | |||
| // | |||
| // SPDX-License-Identifier: Apache-2.0 | |||
|
|
|||
| #include "dprint_fixture.hpp" | |||
| #include "debug_tools_fixtures.hpp" | |||
There was a problem hiding this comment.
debug_tools_fixtures.hpp file not found
|
|
||
| #include "tests/tt_metal/tt_metal/unit_tests_common/common/common_fixture.hpp" | ||
| #include "dispatch_fixture.hpp" |
There was a problem hiding this comment.
dispatch_fixture.hpp file not found
tests/tt_metal/tt_metal/unit_tests_fast_dispatch/common/command_queue_fixture.hpp
Outdated
Show resolved
Hide resolved
| @@ -2,7 +2,7 @@ | |||
| // | |||
| // SPDX-License-Identifier: Apache-2.0 | |||
|
|
|||
| #include "watcher_fixture.hpp" | |||
| #include "debug_tools_fixtures.hpp" | |||
There was a problem hiding this comment.
debug_tools_fixtures.hpp file not found
| @@ -6,7 +6,7 @@ | |||
| #include <functional> | |||
| #include <random> | |||
|
|
|||
| #include "tests/tt_metal/tt_metal/unit_tests_common/common/common_fixture.hpp" | |||
| #include "dispatch_fixture.hpp" | |||
There was a problem hiding this comment.
dispatch_fixture.hpp file not found
| @@ -2,7 +2,7 @@ | |||
| // | |||
| // SPDX-License-Identifier: Apache-2.0 | |||
|
|
|||
| #include "watcher_fixture.hpp" | |||
| #include "debug_tools_fixtures.hpp" | |||
There was a problem hiding this comment.
debug_tools_fixtures.hpp file not found
| @@ -2,7 +2,7 @@ | |||
| // | |||
| // SPDX-License-Identifier: Apache-2.0 | |||
|
|
|||
| #include "watcher_fixture.hpp" | |||
| #include "debug_tools_fixtures.hpp" | |||
There was a problem hiding this comment.
debug_tools_fixtures.hpp file not found
| @@ -4,13 +4,13 @@ | |||
|
|
|||
| // This file contains dispatch tests that are (generally) dispatch mode agnostic | |||
|
|
|||
| #include "tests/tt_metal/tt_metal/unit_tests_common/common/common_fixture.hpp" | |||
| #include "dispatch_fixture.hpp" | |||
There was a problem hiding this comment.
dispatch_fixture.hpp file not found
| @@ -6,7 +6,7 @@ | |||
| #include <functional> | |||
| #include <random> | |||
|
|
|||
| #include "tests/tt_metal/tt_metal/unit_tests_common/common/common_fixture.hpp" | |||
| #include "dispatch_fixture.hpp" | |||
There was a problem hiding this comment.
dispatch_fixture.hpp file not found
| @@ -3,10 +3,11 @@ | |||
| // SPDX-License-Identifier: Apache-2.0 | |||
|
|
|||
| #include "bit_utils.h" | |||
| #include "host_fixture.hpp" | |||
There was a problem hiding this comment.
host_fixture.hpp file not found
tests/tt_metal/tt_metal/unit_tests/common/multi_device_fixture.hpp
Outdated
Show resolved
Hide resolved
| @@ -4,10 +4,6 @@ | |||
|
|
|||
| #include <gtest/gtest.h> | |||
There was a problem hiding this comment.
gtest/gtest.h file not found
|
|
||
| #pragma once | ||
|
|
||
| #include <gtest/gtest.h> |
There was a problem hiding this comment.
gtest/gtest.h file not found
| @@ -2,7 +2,7 @@ | |||
| // | |||
| // SPDX-License-Identifier: Apache-2.0 | |||
|
|
|||
| #include "watcher_fixture.hpp" | |||
| #include "debug_tools_fixtures.hpp" | |||
There was a problem hiding this comment.
debug_tools_fixtures.hpp file not found
...l/tt_metal/unit_tests_fast_dispatch_single_chip_multi_queue/common/command_queue_fixture.hpp
Show resolved
Hide resolved
tests/tt_metal/tt_metal/unit_tests/fast_dispatch_kernels/test_write_host.cpp
Outdated
Show resolved
Hide resolved
| @@ -2,7 +2,7 @@ | |||
| // | |||
| // SPDX-License-Identifier: Apache-2.0 | |||
|
|
|||
| #include "watcher_fixture.hpp" | |||
| #include "debug_tools_fixtures.hpp" | |||
There was a problem hiding this comment.
debug_tools_fixtures.hpp file not found
| @@ -4,8 +4,8 @@ | |||
|
|
|||
| #include <cstddef> | |||
| #include <cstdint> | |||
| #include <memory> | |||
| #include "command_queue_fixture.hpp" | |||
There was a problem hiding this comment.
command_queue_fixture.hpp file not found
| @@ -2,7 +2,7 @@ | |||
| // | |||
| // SPDX-License-Identifier: Apache-2.0 | |||
|
|
|||
| #include "tests/tt_metal/tt_metal/unit_tests_common/common/common_fixture.hpp" | |||
| #include "dispatch_fixture.hpp" | |||
There was a problem hiding this comment.
dispatch_fixture.hpp file not found
| @@ -3,7 +3,7 @@ | |||
| // SPDX-License-Identifier: Apache-2.0 | |||
|
|
|||
| #include "common/bfloat16.hpp" | |||
| #include "dprint_fixture.hpp" | |||
| #include "debug_tools_fixtures.hpp" | |||
There was a problem hiding this comment.
debug_tools_fixtures.hpp file not found
| @@ -1,7 +1,7 @@ | |||
| // SPDX-FileCopyrightText: © 2023 Tenstorrent Inc. | |||
| // | |||
| // SPDX-License-Identifier: Apache-2.0 | |||
| #include "tests/tt_metal/tt_metal/unit_tests_common/common/common_fixture.hpp" | |||
| #include "dispatch_fixture.hpp" | |||
There was a problem hiding this comment.
dispatch_fixture.hpp file not found
| @@ -1,7 +1,7 @@ | |||
| // SPDX-FileCopyrightText: © 2023 Tenstorrent Inc. | |||
| // | |||
| // SPDX-License-Identifier: Apache-2.0 | |||
| #include "dprint_fixture.hpp" | |||
| #include "debug_tools_fixtures.hpp" | |||
There was a problem hiding this comment.
debug_tools_fixtures.hpp file not found
| @@ -6,7 +6,7 @@ | |||
| #include <functional> | |||
| #include <random> | |||
|
|
|||
| #include "tests/tt_metal/tt_metal/unit_tests_common/common/common_fixture.hpp" | |||
| #include "dispatch_fixture.hpp" | |||
There was a problem hiding this comment.
dispatch_fixture.hpp file not found
| @@ -2,7 +2,7 @@ | |||
| // | |||
| // SPDX-License-Identifier: Apache-2.0 | |||
|
|
|||
| #include "dprint_fixture.hpp" | |||
| #include "debug_tools_fixtures.hpp" | |||
There was a problem hiding this comment.
debug_tools_fixtures.hpp file not found
| @@ -1,7 +1,7 @@ | |||
| // SPDX-FileCopyrightText: © 2023 Tenstorrent Inc. | |||
| // | |||
| // SPDX-License-Identifier: Apache-2.0 | |||
| #include "dprint_fixture.hpp" | |||
| #include "debug_tools_fixtures.hpp" | |||
There was a problem hiding this comment.
debug_tools_fixtures.hpp file not found
| @@ -2,7 +2,7 @@ | |||
| // | |||
| // SPDX-License-Identifier: Apache-2.0 | |||
|
|
|||
| #include "watcher_fixture.hpp" | |||
| #include "debug_tools_fixtures.hpp" | |||
There was a problem hiding this comment.
debug_tools_fixtures.hpp file not found
| @@ -2,7 +2,7 @@ | |||
| // | |||
| // SPDX-License-Identifier: Apache-2.0 | |||
|
|
|||
| #include "dprint_fixture.hpp" | |||
| #include "debug_tools_fixtures.hpp" | |||
There was a problem hiding this comment.
debug_tools_fixtures.hpp file not found
| @@ -2,7 +2,7 @@ | |||
| // | |||
| // SPDX-License-Identifier: Apache-2.0 | |||
|
|
|||
| #include "dprint_fixture.hpp" | |||
| #include "debug_tools_fixtures.hpp" | |||
There was a problem hiding this comment.
debug_tools_fixtures.hpp file not found
| @@ -4,7 +4,7 @@ | |||
|
|
|||
| #include <gtest/gtest.h> | |||
There was a problem hiding this comment.
gtest/gtest.h file not found
| @@ -3,7 +3,7 @@ | |||
| // SPDX-License-Identifier: Apache-2.0 | |||
|
|
|||
| #include "llrt/rtoptions.hpp" | |||
| #include "watcher_fixture.hpp" | |||
| #include "debug_tools_fixtures.hpp" | |||
There was a problem hiding this comment.
debug_tools_fixtures.hpp file not found
|
|
||
| // A version of CommonFixture with watcher enabled | ||
| class WatcherFixture: public CommonFixture { | ||
| #include "dispatch_fixture.hpp" |
There was a problem hiding this comment.
dispatch_fixture.hpp file not found
| @@ -2,7 +2,7 @@ | |||
| // | |||
| // SPDX-License-Identifier: Apache-2.0 | |||
|
|
|||
| #include "dprint_fixture.hpp" | |||
| #include "debug_tools_fixtures.hpp" | |||
There was a problem hiding this comment.
debug_tools_fixtures.hpp file not found
| @@ -2,7 +2,7 @@ | |||
| // | |||
| // SPDX-License-Identifier: Apache-2.0 | |||
|
|
|||
| #include "dprint_fixture.hpp" | |||
| #include "debug_tools_fixtures.hpp" | |||
There was a problem hiding this comment.
debug_tools_fixtures.hpp file not found
| @@ -6,7 +6,7 @@ | |||
| #include <functional> | |||
| #include <random> | |||
|
|
|||
| #include "tests/tt_metal/tt_metal/unit_tests_common/common/common_fixture.hpp" | |||
| #include "dispatch_fixture.hpp" | |||
There was a problem hiding this comment.
dispatch_fixture.hpp file not found
| @@ -3,6 +3,7 @@ | |||
| // SPDX-License-Identifier: Apache-2.0 | |||
|
|
|||
| #include "command_queue_fixture.hpp" | |||
There was a problem hiding this comment.
command_queue_fixture.hpp file not found
| @@ -6,7 +6,7 @@ | |||
| #include <functional> | |||
| #include <random> | |||
|
|
|||
| #include "tests/tt_metal/tt_metal/unit_tests_common/common/common_fixture.hpp" | |||
| #include "dispatch_fixture.hpp" | |||
There was a problem hiding this comment.
dispatch_fixture.hpp file not found
| @@ -6,7 +6,7 @@ | |||
| #include <functional> | |||
| #include <random> | |||
|
|
|||
| #include "tests/tt_metal/tt_metal/unit_tests_common/common/common_fixture.hpp" | |||
| #include "dispatch_fixture.hpp" | |||
There was a problem hiding this comment.
dispatch_fixture.hpp file not found
There was a problem hiding this comment.
Looks good so far!
From going through this a couple categories that could help define the structure stick out to me:
- a bring-up / basic section in the unit_test dir and unit_test_common. These tests would go through minimal Metal RT codepath but what we need working before we try running more complex codepaths
- LLK section, LLK team has been adding parameterized single core slow dispatch tests under "compute"
- API constructs section: tests that create/initialize rt args, semaphores, circular buffers, kernels, programs, allocator (these wouldn't necessarily run anything on device but serve as the base complexity level of RT / flush out API rules)
- Fast dispatch tests: single chip/multi-chip enqueue APIs
- Launching programs: these can be incrementally complex as Paul mentioned (test loading a kernel, tests that test across eth/tensix, complex use cases)
| @@ -81,7 +81,7 @@ bool dram_ping( | |||
| } | |||
| } // namespace unit_tests::basic::device | |||
|
|
|||
| TEST_F(BasicFixture, SingleDeviceHarvestingPrints) { | |||
| TEST_F(BasicFixture, TensixSingleDeviceHarvestingPrints) { | |||
There was a problem hiding this comment.
I think this file should be prepended with test -> test_device.cpp. Also, I think we should rename BasicFixture -> BasicDeviceFixture. Most of the tests that are in this file should use that fixture because they are the basic functionalities that we expect when bringing up/running on device.
Maybe "basic" -> "bringup" and then we can have a bring-up device folder (these could be the least complex tests with minimal RT features but basic host <-> device interactions that are in more complex codepaths
| @@ -102,7 +102,7 @@ void try_creating_more_than_max_num_semaphores( | |||
|
|
|||
| } // namespace unit_tests::initialize_semaphores | |||
|
|
|||
| TEST_F(DeviceFixture, InitializeLegalSemaphores) { | |||
| TEST_F(DeviceFixture, TensixInitializeLegalSemaphores) { | |||
There was a problem hiding this comment.
nit: prefix test file with test_*
I know the structure isn't finalized but I don't think this belongs in basic/bringup folder. We should have a separate dir where we test API constructs
| @@ -168,7 +168,7 @@ bool verify_results( | |||
| } | |||
|
|
|||
| // Write unique and common runtime args to device and readback to verify written correctly. | |||
| TEST_F(DeviceFixture, LegallyModifyRTArgsDataMovement) { | |||
| TEST_F(DeviceFixture, TensixLegallyModifyRTArgsDataMovement) { | |||
There was a problem hiding this comment.
these should be moved to api constructs
| @@ -72,7 +72,7 @@ void read_translation_table (Device* device, CoreCoord logical_node, std::vector | |||
|
|
|||
|
|
|||
|
|
|||
| TEST_F(BasicFixture, VerifyNocNodeIDs) { | |||
| TEST_F(BasicFixture, TensixVerifyNocNodeIDs) { | |||
There was a problem hiding this comment.
this definitely belongs in basic or bringup
| @@ -289,11 +289,11 @@ void run_single_core_broadcast(tt_metal::Device* device, const BroadcastConfig& | |||
| } | |||
| } | |||
|
|
|||
| class BroadcastParametrizedDeviceFixture : public DeviceFixture, | |||
| class BroadcastParameterizedDeviceFixture : public DeviceFixture, | |||
There was a problem hiding this comment.
currently everything in unit_tests/compute are meant to be simple single core LLK test. I think we should have a dedicated LLK or Compute fixture for all the tests in this directory that currently run using DeviceFixture
| @@ -372,7 +372,7 @@ bool reader_datacopy_writer(tt_metal::Device* device, const ReaderDatacopyWriter | |||
| } | |||
| } // namespace unit_tests::dram::direct | |||
|
|
|||
| TEST_F(DeviceFixture, SingleCoreDirectDramReaderOnly) { | |||
| TEST_F(DeviceFixture, TensixSingleCoreDirectDramReaderOnly) { | |||
There was a problem hiding this comment.
this should be moved to the bring-up folder
| @@ -2,7 +2,7 @@ | |||
| // | |||
| // SPDX-License-Identifier: Apache-2.0 | |||
|
|
|||
| #include "tests/tt_metal/tt_metal/unit_tests_common/common/common_fixture.hpp" | |||
| #include "dispatch_fixture.hpp" | |||
There was a problem hiding this comment.
this should be moved to api constructs dir
sagarwalTT
force-pushed
the
sagarwal/fix-test-naming
branch
from
24700c2
to
294ee25
Compare
| @@ -2,7 +2,7 @@ | |||
| // | |||
| // SPDX-License-Identifier: Apache-2.0 | |||
|
|
|||
| #include "watcher_fixture.hpp" | |||
| #include "debug_tools_fixture.hpp" | |||
There was a problem hiding this comment.
debug_tools_fixture.hpp file not found
| @@ -2,7 +2,7 @@ | |||
| // | |||
| // SPDX-License-Identifier: Apache-2.0 | |||
|
|
|||
| #include "dprint_fixture.hpp" | |||
| #include "debug_tools_fixture.hpp" | |||
There was a problem hiding this comment.
debug_tools_fixture.hpp file not found
| @@ -3,7 +3,7 @@ | |||
| // SPDX-License-Identifier: Apache-2.0 | |||
|
|
|||
| #include "common/bfloat16.hpp" | |||
| #include "dprint_fixture.hpp" | |||
| #include "debug_tools_fixture.hpp" | |||
There was a problem hiding this comment.
debug_tools_fixture.hpp file not found
| @@ -2,7 +2,7 @@ | |||
| // | |||
| // SPDX-License-Identifier: Apache-2.0 | |||
|
|
|||
| #include "watcher_fixture.hpp" | |||
| #include "debug_tools_fixture.hpp" | |||
There was a problem hiding this comment.
debug_tools_fixture.hpp file not found
| @@ -2,7 +2,7 @@ | |||
| // | |||
| // SPDX-License-Identifier: Apache-2.0 | |||
|
|
|||
| #include "dprint_fixture.hpp" | |||
| #include "debug_tools_fixture.hpp" | |||
There was a problem hiding this comment.
debug_tools_fixture.hpp file not found
|
|
||
| // A version of CommonFixture with watcher enabled | ||
| class WatcherFixture: public CommonFixture { | ||
| #include "dispatch_fixture.hpp" |
There was a problem hiding this comment.
dispatch_fixture.hpp file not found
| @@ -1,7 +1,7 @@ | |||
| // SPDX-FileCopyrightText: © 2023 Tenstorrent Inc. | |||
| // | |||
| // SPDX-License-Identifier: Apache-2.0 | |||
| #include "dprint_fixture.hpp" | |||
| #include "debug_tools_fixture.hpp" | |||
There was a problem hiding this comment.
debug_tools_fixture.hpp file not found
| @@ -2,7 +2,7 @@ | |||
| // | |||
| // SPDX-License-Identifier: Apache-2.0 | |||
|
|
|||
| #include "dprint_fixture.hpp" | |||
| #include "debug_tools_fixture.hpp" | |||
There was a problem hiding this comment.
debug_tools_fixture.hpp file not found
| @@ -2,7 +2,7 @@ | |||
| // | |||
| // SPDX-License-Identifier: Apache-2.0 | |||
|
|
|||
| #include "dprint_fixture.hpp" | |||
| #include "debug_tools_fixture.hpp" | |||
There was a problem hiding this comment.
debug_tools_fixture.hpp file not found
| @@ -2,7 +2,7 @@ | |||
| // | |||
| // SPDX-License-Identifier: Apache-2.0 | |||
|
|
|||
| #include "watcher_fixture.hpp" | |||
| #include "debug_tools_fixture.hpp" | |||
There was a problem hiding this comment.
debug_tools_fixture.hpp file not found
|
|
||
| #pragma once | ||
|
|
||
| #include <gtest/gtest.h> |
There was a problem hiding this comment.
gtest/gtest.h file not found
| @@ -1,7 +1,7 @@ | |||
| // SPDX-FileCopyrightText: © 2023 Tenstorrent Inc. | |||
| // | |||
| // SPDX-License-Identifier: Apache-2.0 | |||
| #include "dprint_fixture.hpp" | |||
| #include "debug_tools_fixture.hpp" | |||
There was a problem hiding this comment.
debug_tools_fixture.hpp file not found
| @@ -2,7 +2,7 @@ | |||
| // | |||
| // SPDX-License-Identifier: Apache-2.0 | |||
|
|
|||
| #include "watcher_fixture.hpp" | |||
| #include "debug_tools_fixture.hpp" | |||
There was a problem hiding this comment.
debug_tools_fixture.hpp file not found
| @@ -2,7 +2,7 @@ | |||
| // | |||
| // SPDX-License-Identifier: Apache-2.0 | |||
|
|
|||
| #include "watcher_fixture.hpp" | |||
| #include "debug_tools_fixture.hpp" | |||
There was a problem hiding this comment.
debug_tools_fixture.hpp file not found
| @@ -2,7 +2,7 @@ | |||
| // | |||
| // SPDX-License-Identifier: Apache-2.0 | |||
|
|
|||
| #include "watcher_fixture.hpp" | |||
| #include "debug_tools_fixture.hpp" | |||
There was a problem hiding this comment.
debug_tools_fixture.hpp file not found
| @@ -3,7 +3,7 @@ | |||
| // SPDX-License-Identifier: Apache-2.0 | |||
|
|
|||
| #include "llrt/rtoptions.hpp" | |||
| #include "watcher_fixture.hpp" | |||
| #include "debug_tools_fixture.hpp" | |||
There was a problem hiding this comment.
debug_tools_fixture.hpp file not found
|
|
||
| #pragma once | ||
|
|
||
| #include "gtest/gtest.h" |
There was a problem hiding this comment.
gtest/gtest.h file not found
| @@ -2,7 +2,7 @@ | |||
| // | |||
| // SPDX-License-Identifier: Apache-2.0 | |||
|
|
|||
| #include "dprint_fixture.hpp" | |||
| #include "debug_tools_fixture.hpp" | |||
There was a problem hiding this comment.
debug_tools_fixture.hpp file not found
|
|
||
| #pragma once | ||
|
|
||
| #include <gtest/gtest.h> |
There was a problem hiding this comment.
gtest/gtest.h file not found
| @@ -2,7 +2,7 @@ | |||
| // | |||
| // SPDX-License-Identifier: Apache-2.0 | |||
|
|
|||
| #include "dprint_fixture.hpp" | |||
| #include "debug_tools_fixture.hpp" | |||
There was a problem hiding this comment.
debug_tools_fixture.hpp file not found
sagarwalTT
force-pushed
the
sagarwal/fix-test-naming
branch
10 times, most recently
from
f1e99a8
to
b7996aa
Compare
sagarwalTT
force-pushed
the
sagarwal/fix-test-naming
branch
11 times, most recently
from
07c8875
to
aeec1d0
Compare
sagarwalTT
force-pushed
the
sagarwal/fix-test-naming
branch
from
aeec1d0
to
67e5881
Compare
| class CommandQueueFixture : public DispatchFixture { | ||
| protected: | ||
| tt::tt_metal::Device* device_; | ||
| void SetUp() override { |
There was a problem hiding this comment.
Generally c'tor / d'tor are preferred over SetUp / TearDown. https://google.github.io/googletest/faq.html#CtorVsSetUp
Is it required here because CloseDevice isn't noexcept (why isn't it noexcept? 🤔 )
| @@ -254,7 +254,7 @@ fast dispatch, you can | |||
| ``` | |||
| 2. Run the test: | |||
| ``` | |||
| ./build/test/tt_metal/unit_tests_fast_dispatch --gtest_filter="CommonFixture.DRAMLoopbackSingleCore" | |||
| ./build/test/tt_metal/ --gtest_filter="DispatchFixture.TensixDRAMLoopbackSingleCore" | |||
There was a problem hiding this comment.
Looks like this is missing the filename of the binary.
| set(UNIT_TESTS_DISPATCH_BUFFER_SRC | ||
| ${CMAKE_CURRENT_SOURCE_DIR}/test_EnqueueWriteBuffer_and_EnqueueReadBuffer.cpp | ||
| ${CMAKE_CURRENT_SOURCE_DIR}/test_sub_device.cpp | ||
| CACHE INTERNAL |
There was a problem hiding this comment.
Why is this a cache variable?
Ticket
#14690
Checklist