diff mbox series

[v2,6/7] cmake: use test names instead of full paths

Message ID 41228df1b469d9a79f3278fe8c1ca37082600669.1695070468.git.gitgitgadget@gmail.com (mailing list archive)
State New, archived
Headers show
Series CMake(Visual C) support for js/doc-unit-tests | expand

Commit Message

Johannes Schindelin Sept. 18, 2023, 8:54 p.m. UTC 
From: Johannes Schindelin <johannes.schindelin@gmx.de>

The primary purpose of Git's CMake definition is to allow developing Git
in Visual Studio. As part of that, the CTest feature allows running
individual test scripts conveniently in Visual Studio's Test Explorer.

However, this Test Explorer's design targets object-oriented languages
and therefore expects the test names in the form
`<namespace>.<class>.<testname>`. And since we specify the full path
of the test scripts instead, including the ugly `/.././t/` part, these
dots confuse the Test Explorer and it uses a large part of the path as
"namespace".

Let's just use `t.<name>` instead. This still adds an ugly "Empty
Namespace" layer by default, but at least the ugly absolute path is now
gone.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 contrib/buildsystems/CMakeLists.txt | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

Comments

Phillip Wood Sept. 22, 2023, 2:37 p.m. UTC | #1 
Hi Johannes

On 18/09/2023 21:54, Johannes Schindelin via GitGitGadget wrote:
> From: Johannes Schindelin <johannes.schindelin@gmx.de>
> 
> The primary purpose of Git's CMake definition is to allow developing Git
> in Visual Studio. As part of that, the CTest feature allows running
> individual test scripts conveniently in Visual Studio's Test Explorer.
> 
> However, this Test Explorer's design targets object-oriented languages
> and therefore expects the test names in the form
> `<namespace>.<class>.<testname>`. And since we specify the full path
> of the test scripts instead, including the ugly `/.././t/` part, these
> dots confuse the Test Explorer and it uses a large part of the path as
> "namespace".
> 
> Let's just use `t.<name>` instead. This still adds an ugly "Empty
> Namespace" layer by default, but at least the ugly absolute path is now
> gone.

That does sound like a worthwhile improvement. If we used `git.t.<name>` 
would that fix the "Empty Namespace" problem? (probably not worth a 
re-roll on its own)

Best Wishes

Phillip

> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> ---
>   contrib/buildsystems/CMakeLists.txt | 5 +++--
>   1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/contrib/buildsystems/CMakeLists.txt b/contrib/buildsystems/CMakeLists.txt
> index ad197ea433f..ff1a8cc348f 100644
> --- a/contrib/buildsystems/CMakeLists.txt
> +++ b/contrib/buildsystems/CMakeLists.txt
> @@ -1106,13 +1106,14 @@ file(GLOB test_scripts "${CMAKE_SOURCE_DIR}/t/t[0-9]*.sh")
>   
>   #test
>   foreach(tsh ${test_scripts})
> -	add_test(NAME ${tsh}
> +	string(REGEX REPLACE ".*/(.*)\\.sh" "\\1" test_name ${tsh})
> +	add_test(NAME "t.${test_name}"
>   		COMMAND ${SH_EXE} ${tsh} --no-bin-wrappers --no-chain-lint -vx
>   		WORKING_DIRECTORY ${CMAKE_SOURCE_DIR}/t)
>   endforeach()
>   
>   # This test script takes an extremely long time and is known to time out even
>   # on fast machines because it requires in excess of one hour to run
> -set_tests_properties("${CMAKE_SOURCE_DIR}/t/t7112-reset-submodule.sh" PROPERTIES TIMEOUT 4000)
> +set_tests_properties("t.t7112-reset-submodule" PROPERTIES TIMEOUT 4000)
>   
>   endif()#BUILD_TESTING
Johannes Schindelin Sept. 25, 2023, 9:06 a.m. UTC | #2 
Hi Phillip,

On Fri, 22 Sep 2023, Phillip Wood wrote:

> On 18/09/2023 21:54, Johannes Schindelin via GitGitGadget wrote:
> > From: Johannes Schindelin <johannes.schindelin@gmx.de>
> >
> > The primary purpose of Git's CMake definition is to allow developing Git
> > in Visual Studio. As part of that, the CTest feature allows running
> > individual test scripts conveniently in Visual Studio's Test Explorer.
> >
> > However, this Test Explorer's design targets object-oriented languages
> > and therefore expects the test names in the form
> > `<namespace>.<class>.<testname>`. And since we specify the full path
> > of the test scripts instead, including the ugly `/.././t/` part, these
> > dots confuse the Test Explorer and it uses a large part of the path as
> > "namespace".
> >
> > Let's just use `t.<name>` instead. This still adds an ugly "Empty
> > Namespace" layer by default, but at least the ugly absolute path is now
> > gone.
>
> That does sound like a worthwhile improvement. If we used `git.t.<name>` would
> that fix the "Empty Namespace" problem? (probably not worth a re-roll on its
> own)

Turns out I did not play around with this enough. If I use `t.suite.` as a
prefix, it makes the empty name space go away.

I'll do that, then.

Ciao,
Johannes
diff mbox series

Patch

diff --git a/contrib/buildsystems/CMakeLists.txt b/contrib/buildsystems/CMakeLists.txt
index ad197ea433f..ff1a8cc348f 100644
--- a/contrib/buildsystems/CMakeLists.txt
+++ b/contrib/buildsystems/CMakeLists.txt
@@ -1106,13 +1106,14 @@  file(GLOB test_scripts "${CMAKE_SOURCE_DIR}/t/t[0-9]*.sh")
 
 #test
 foreach(tsh ${test_scripts})
-	add_test(NAME ${tsh}
+	string(REGEX REPLACE ".*/(.*)\\.sh" "\\1" test_name ${tsh})
+	add_test(NAME "t.${test_name}"
 		COMMAND ${SH_EXE} ${tsh} --no-bin-wrappers --no-chain-lint -vx
 		WORKING_DIRECTORY ${CMAKE_SOURCE_DIR}/t)
 endforeach()
 
 # This test script takes an extremely long time and is known to time out even
 # on fast machines because it requires in excess of one hour to run
-set_tests_properties("${CMAKE_SOURCE_DIR}/t/t7112-reset-submodule.sh" PROPERTIES TIMEOUT 4000)
+set_tests_properties("t.t7112-reset-submodule" PROPERTIES TIMEOUT 4000)
 
 endif()#BUILD_TESTING