CMake : Refactor Mbed OS drivers UNITTESTS #14285

rajkan01 · 2021-02-15T11:37:31Z

Summary of changes

Created a CMake stubs library.
Created CMake fake watchdog and PWM out the library.
Added CMakeLists in Watchdog and PWM unittest to build with the respective watchdog and PWM fake library.

Impact of changes

None.

Migration actions required

None.

Documentation

None.

Pull request type

[] Patch update (Bug fix / Target update / Docs update / Test update / Refactor)
[x] Feature update (New feature / Functionality change / New API)
[] Major update (Breaking change E.g. Return code change / API behaviour change)

Test results

[] No Tests required for this change (E.g docs only update)
[x] Covered by existing mbed-os tests (Greentea or Unittest)
[] Tests / results supplied as part of this PR

Reviewers

@0xc0170 @hugueskamba @paul-szczepanek-arm @pan-

ciarmcom · 2021-02-15T12:00:17Z

@rajkan01, thank you for your changes.
@0xc0170 @pan- @hugueskamba @paul-szczepanek-arm @ARMmbed/mbed-os-maintainers please review.

UNITTESTS/fakes/CMakeLists.txt

UNITTESTS/stubs/CMakeLists.txt

0xc0170 · 2021-02-15T14:18:23Z

drivers/tests/UNITTESTS/PwmOut/CMakeLists.txt

@@ -0,0 +1,22 @@
+# Copyright (c) 2020 ARM Limited. All rights reserved.
+# SPDX-License-Identifier: Apache-2.0
+cmake_minimum_required(VERSION 3.0.2)


shouldn't this be in our main unittests cmake?

I noticed this one is 3.0.2 other is 3.19?

UNITTESTS/fakes/drivers/watchdog/CMakeLists.txt

hugueskamba · 2021-02-16T10:40:28Z

UNITTESTS/fakes/CMakeLists.txt

@@ -0,0 +1,11 @@
+# Copyright (c) 2020 ARM Limited. All rights reserved.


Suggested change

# Copyright (c) 2020 ARM Limited. All rights reserved.

# Copyright (c) 2021 ARM Limited. All rights reserved.

UNITTESTS/fakes/CMakeLists.txt

UNITTESTS/fakes/drivers/CMakeLists.txt

tools/cmake/mbed_unittest.cmake

rajkan01 · 2021-02-16T17:12:04Z

@paul-szczepanek-arm @hugueskamba @0xc0170 I have addressed all the review comments, please re-review

tools/cmake/mbed_unittest.cmake

0xc0170 · 2021-02-17T09:29:20Z

drivers/tests/UNITTESTS/Watchdog/CMakeLists.txt

+
+target_link_libraries(${TEST_NAME}
+    PRIVATE
+        mbed-os-headers


we should follow the naming we agreed, mbed prefix only. Or is this mbed-os (could be mbed-baremetal ?) ?

UNITTESTS/fakes/drivers/CMakeLists.txt

- Created a CMake stubs library. - Created CMake fake watchdog and PWM out the library. - Added CMakeLists in Watchdog and PWM unittest to build with the respective watchdog and PWM fake library.

UNITTESTS/stubs/hal/CMakeLists.txt

paul-szczepanek-arm · 2021-02-19T09:46:50Z

UNITTESTS/stubs/connectivity/CMakeLists.txt

+        SocketAddress_stub.cpp
+        aes_stub.c
+        cipher_stub.c
+        #AT_CellularContext_stub.cpp


this is work in progress as I add connectivity tests

paul-szczepanek-arm · 2021-02-19T09:49:56Z

UNITTESTS/stubs/storage/CMakeLists.txt

+        FileHandle_stub.cpp
+        FlashSimBlockDevice_stub.cpp
+        HeapBlockDevice_stub.cpp
+        #MBRBlockDevice_stub.cpp


appears broken and unused - this needs to be checked if any tests actually use it

paul-szczepanek-arm · 2021-02-19T09:51:26Z

I kept stubs headers in a separate dir and as a separate lib as some tests use the stub headers directly

- Remove redundant file

- Fix connectivity build issue of miss to link aginst gtest

0xc0170 · 2021-02-22T10:37:50Z

CMakeLists.txt

@@ -5,6 +5,18 @@

 cmake_minimum_required(VERSION 3.19.0 FATAL_ERROR)

+option(UNITTESTS "Run unit tests only." OFF)
+
+if(UNITTESTS)


Should we be more specific to avoid clashes with a users (if they want to run their unittests only, not ours and they might have the same macro ? https://cliutils.gitlab.io/modern-cmake/chapters/testing.html (using BUILD_TESTING and project name):

if(CMAKE_PROJECT_NAME STREQUAL PROJECT_NAME AND BUILD_TESTING) add_subdirectory(tests) endif()

BUILD_TESTING is already provided, shall we use it ? Or rather to use MBED_BUILD_TESTING to be able to distinguish build testing in Mbed OS only or everywhere (if everywhere, a user would set MBED_BUILD_TESTING and BUILD_TESTING) ? Reading some issues around this (enable_testing is a global thing), I would go with MBED_BUILD_TESTING.

oh yeah, if there's already an option for that then that'd be perfect

0xc0170 · 2021-02-22T10:45:38Z

UNITTESTS/CMakeLists.txt

-    message(WARNING "No test source files found for ${TEST_SUITE_NAME}.\n")
-  endif(unittest-test-sources)
-endforeach(testfile)
+macro( mbed_add_all_subdirectories subdirectories_list )


Where is this used , I can't find a reference.

it's no longer used since we're not globbing the dirs any more

lets remove it

I have used this macro in storage unittest refactoring #14319 as we have more number of subdirectories to add

OK lets look at 14319 once t his get in and we can get back to this macro usability and if it should stay or be removed.

0xc0170 · 2021-02-22T10:47:12Z

CMakeLists.txt

+add_subdirectory(drivers/tests/UNITTESTS)
+add_subdirectory(connectivity/cellular/tests/UNITTESTS)
+
+else()


do we need else clause here ? A project usually just do if build testing and add tests as we do. What's the reason for else clause?

without it won't it also try to configure and build mbed-os?

yes, they need to be two different projects because we're doing cross compilation.
For our project we had it setup like this to start with and eventually moved to a completely unrelated project. We had to duplicate a few things but in the end it was much easier.

Would it build if you do not link to Mbed Os at all?

For our project we had it setup like this to start with and eventually moved to a completely unrelated project. We had to duplicate a few things but in the end it was much easier.

you also had either tests or the component ? Can you illustrate?

Would it build if you do not link to Mbed Os at all?

Good question, I must do some testing.

The issue we ran into was that you actually need two different compiles for unit testing and hardware and cmake cannot handle this case.

I'll create an example and report here.

CMakeLists.txt

mergify bot added the release-version: feature branch label Feb 15, 2021

rajkan01 changed the title ~~CMake : Refactor Mbed OS UNITTESTS~~ CMake : Refactor Mbed OS drivers UNITTESTS Feb 15, 2021

ciarmcom requested review from 0xc0170, hugueskamba, pan-, paul-szczepanek-arm and a team February 15, 2021 12:00

ciarmcom added the needs: review label Feb 15, 2021

rajkan01 force-pushed the unittest_refactor branch 2 times, most recently from ab9a7df to a94191b Compare February 15, 2021 13:22

0xc0170 reviewed Feb 15, 2021

View reviewed changes

rajkan01 force-pushed the unittest_refactor branch from a94191b to 1d1c02e Compare February 15, 2021 14:48

rajkan01 requested a review from 0xc0170 February 15, 2021 14:48

paul-szczepanek-arm reviewed Feb 15, 2021

View reviewed changes

UNITTESTS/fakes/drivers/watchdog/CMakeLists.txt Outdated Show resolved Hide resolved

hugueskamba reviewed Feb 16, 2021

View reviewed changes

rajkan01 force-pushed the unittest_refactor branch from 1d1c02e to 25d199f Compare February 16, 2021 14:47

hugueskamba reviewed Feb 16, 2021

View reviewed changes

tools/cmake/mbed_unittest.cmake Outdated Show resolved Hide resolved

rajkan01 force-pushed the unittest_refactor branch 3 times, most recently from 8a2fa37 to 4bced25 Compare February 16, 2021 17:09

rajkan01 requested review from paul-szczepanek-arm and hugueskamba February 16, 2021 17:13

rajkan01 mentioned this pull request Feb 16, 2021

CMake: Refactor Mbed OS Platform UNITTESTS #14294

Merged

0xc0170 previously requested changes Feb 17, 2021

View reviewed changes

mergify bot added needs: work and removed needs: review labels Feb 17, 2021

paul-szczepanek-arm reviewed Feb 18, 2021

View reviewed changes

UNITTESTS/fakes/drivers/CMakeLists.txt Outdated Show resolved Hide resolved

CMake : Refactor Mbed OS UNITTESTS

5e7f992

- Created a CMake stubs library. - Created CMake fake watchdog and PWM out the library. - Added CMakeLists in Watchdog and PWM unittest to build with the respective watchdog and PWM fake library.

paul-szczepanek-arm added 2 commits February 19, 2021 08:37

don't include bare cmsis due to conflicts

cca373a

add first cellular unittest

7686450

paul-szczepanek-arm reviewed Feb 19, 2021

View reviewed changes

UNITTESTS/stubs/hal/CMakeLists.txt Show resolved Hide resolved

paul-szczepanek-arm reviewed Feb 19, 2021

View reviewed changes

paul-szczepanek-arm added 2 commits February 19, 2021 17:05

add all connectivity stubs

8e2824a

add connectivity framework unittests

ccb2841

rajkan01 force-pushed the unittest_refactor branch from 3339819 to 44be976 Compare February 22, 2021 10:51

- Add missing new line

5a27f3b

- Remove redundant file

rajkan01 force-pushed the unittest_refactor branch from 44be976 to 5a27f3b Compare February 22, 2021 10:56

- Fix CMake project missing warning

0a915fa

- Fix connectivity build issue of miss to link aginst gtest

rajkan01 force-pushed the unittest_refactor branch from b985936 to 0a915fa Compare February 22, 2021 11:26

0xc0170 reviewed Feb 22, 2021

View reviewed changes

CMakeLists.txt Outdated Show resolved Hide resolved

rajkan01 mentioned this pull request Feb 22, 2021

CMake: Refactor storage unittest cmake #14319

Merged

rajkan01 requested a review from 0xc0170 February 22, 2021 16:03

rajkan01 force-pushed the unittest_refactor branch from e5b5662 to d7cd47f Compare February 22, 2021 16:06

Incorporated review comments

29eed7c

rajkan01 force-pushed the unittest_refactor branch from d7cd47f to 29eed7c Compare February 22, 2021 19:04

0xc0170 approved these changes Feb 23, 2021

View reviewed changes

0xc0170 added ready for merge and removed needs: work labels Feb 23, 2021

0xc0170 merged commit 70f6a43 into ARMmbed:feature_unittest_refactor Feb 23, 2021

mergify bot added needs: CI and removed ready for merge labels Feb 23, 2021

rajkan01 mentioned this pull request Mar 31, 2021

CMake: Update unittest doc ARMmbed/mbed-os-5-docs#1427

Merged

rajkan01 mentioned this pull request Apr 26, 2021

CMake: Refactor UNITTESTS CMake #14426

Merged

adbridge removed the needs: CI label May 13, 2021

rajkan01 mentioned this pull request Jun 16, 2021

Move mbed-stubs-platform to the platform directory #14773

Merged

		@@ -0,0 +1,11 @@
		# Copyright (c) 2020 ARM Limited. All rights reserved.

	# Copyright (c) 2020 ARM Limited. All rights reserved.
	# Copyright (c) 2021 ARM Limited. All rights reserved.

CMake : Refactor Mbed OS drivers UNITTESTS #14285

CMake : Refactor Mbed OS drivers UNITTESTS #14285

Uh oh!

Conversation

rajkan01 commented Feb 15, 2021

Summary of changes

Impact of changes

Migration actions required

Documentation

Pull request type

Test results

Reviewers

Uh oh!

ciarmcom commented Feb 15, 2021

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

rajkan01 commented Feb 16, 2021

Uh oh!

Uh oh!

Uh oh!

0xc0170 Feb 17, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

paul-szczepanek-arm commented Feb 19, 2021

Uh oh!

0xc0170 Feb 22, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

0xc0170 Feb 22, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rajkan01 Feb 22, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

0xc0170 Feb 22, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

0xc0170 Feb 17, 2021 •

edited

Loading

0xc0170 Feb 22, 2021 •

edited

Loading

0xc0170 Feb 22, 2021 •

edited

Loading

rajkan01 Feb 22, 2021 •

edited

Loading

0xc0170 Feb 22, 2021 •

edited

Loading