Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CMake: Add CMake to platform greentea test #14072

Merged
merged 4 commits into from
Jan 5, 2021

Conversation

rajkan01
Copy link
Contributor

@rajkan01 rajkan01 commented Dec 18, 2020

Summary of changes

  • Added CMakeLists.txt file to every test suite under the platform so that it can build via CMake command
  • Added the CMake Macro mbed_greentea_cmake_macro with configurable arguments TEST_NAME, TEST_INCLUDE_DIRS, TEST_SOURCES, TEST_REQUIRED_LIBS so that it can be used across all the greentea test suite by passing their includes, sources required libraries.
  • Added CMake macro MBED_BAREMETAL_GREENTEA_TEST which can ON/OFF via CMake command-line argument at the configuration time to select/unselect baremetal greentea test

Impact of changes

None.

Migration actions required

Note

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

Manual testing:
Successfully able to build the test suite with below manual steps

  • Manual steps:
    -- Generated .mbed_build config for DISCO target from blinky example and copied into the atomic test directory.
    -- Manually created test_spec.json, which is required by mbetgt to flash and initiate the test.
    -- Reused host_tests python scripts
    -- Manually created runtests.cmake where it calls mbedgt command via execute_process()
  • Run the test:
    -- CTest -S runtests.cmake

Reviewers

@hugueskamba @0xc0170


@rajkan01 rajkan01 force-pushed the add_cmake_greentea_platform branch 2 times, most recently from babdb45 to 5d4f694 Compare December 18, 2020 11:15
@rajkan01 rajkan01 changed the title CMake: Add platform greentea test with CMake CMake: Add CMake to platform greentea test Dec 18, 2020
@ciarmcom ciarmcom requested review from 0xc0170, hugueskamba and a team December 18, 2020 11:30
@ciarmcom
Copy link
Member

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

Copy link
Collaborator

@hugueskamba hugueskamba left a comment

Choose a reason for hiding this comment

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

Same comments for all files.

@ladislas
Copy link
Contributor

The changes seem to be a lot of copy/paste with only one line changing. Why not use a macro instead? This way, if we need to modify, we can do it only once instead of 25 times.

something like this:

# calling the macro

mbed_config_greentea(TEST_NAME mbed-platform-system-reset EXTRA_SOURCES foo.cpp bar.cpp EXTRA_LIBS mbed-kvstore mbed-xyz)

# the macro

macro(mbed_config_greentea)
    set(options)
    set(singleValueArgs TEST_NAME)
    set(multipleValueArgs EXTRA_SOURCES EXTRA_LIBS)
    cmake_parse_arguments(MBED_CONFIG_GREENTEA "${options}" "${singleValueArgs}"
                          "${multipleValueArgs}" ${ARGN} )

cmake_minimum_required(VERSION 3.19.0 FATAL_ERROR)

set(MBED_PATH ${CMAKE_CURRENT_SOURCE_DIR}/../../../../.. CACHE INTERNAL "")
set(MBED_CONFIG_PATH ${CMAKE_CURRENT_SOURCE_DIR}/.mbedbuild CACHE INTERNAL "")

set(TEST_NAME ${MBED_CONFIG_GREENTEA_TEST_NAME})

include(${MBED_PATH}/tools/cmake/app.cmake)

add_subdirectory(${MBED_PATH} build)

add_executable(${TEST_NAME})

mbed_configure_app_target(${TEST_NAME})

mbed_set_mbed_target_linker_script(${TEST_NAME})

project(${TEST_NAME})

target_sources(${TEST_NAME}
    PRIVATE
        main.cpp
        ${MBED_CONFIG_GREENTEA_EXTRA_SOURCES}
)

if(MBED_BAREMETAL_GREENTEA_TEST)
	set(USE_LIB mbed-baremetal)
else()
	set(USE_LIB mbed-os)
endif()

target_link_libraries(${TEST_NAME}
    PRIVATE
        ${USE_LIB}
        mbed-greentea
		${MBED_CONFIG_GREENTEA_EXTRA_LIBS}
)

mbed_set_post_build(${TEST_NAME})

option(VERBOSE_BUILD "Have a verbose build process")
if(VERBOSE_BUILD)
    set(CMAKE_VERBOSE_MAKEFILE ON)
endif()

endmacro()

Copy link
Contributor

@ladislas ladislas left a comment

Choose a reason for hiding this comment

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

much better with the macro! 👍

tools/cmake/mbed_greentea.cmake Outdated Show resolved Hide resolved
tools/cmake/mbed_greentea.cmake Outdated Show resolved Hide resolved
@rajkan01
Copy link
Contributor Author

@0xc0170 @adbridge It seems to be a problem with pytest in Travis CI setup, please check
image

tools/cmake/mbed_greentea.cmake Outdated Show resolved Hide resolved
tools/cmake/README.md Outdated Show resolved Hide resolved
tools/cmake/README.md Outdated Show resolved Hide resolved
tools/cmake/README.md Outdated Show resolved Hide resolved
tools/cmake/README.md Outdated Show resolved Hide resolved
tools/cmake/README.md Outdated Show resolved Hide resolved
tools/cmake/README.md Outdated Show resolved Hide resolved
tools/cmake/README.md Outdated Show resolved Hide resolved
tools/cmake/README.md Outdated Show resolved Hide resolved
0xc0170
0xc0170 previously approved these changes Jan 5, 2021
Copy link
Contributor

@0xc0170 0xc0170 left a comment

Choose a reason for hiding this comment

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

Two steps required:

  • rebase to fix travis failure
  • commit requested edits from @hugueskamba

We will fix the macro name in the following PR asap.

I approved but should be as request for changes (travis is red anyway).

@mergify mergify bot dismissed stale reviews from hugueskamba and 0xc0170 January 5, 2021 14:48

Pull request has been modified.

@0xc0170
Copy link
Contributor

0xc0170 commented Jan 5, 2021

CI started

@mbed-ci
Copy link

mbed-ci commented Jan 5, 2021

Jenkins CI Test : ✔️ SUCCESS

Build Number: 1 | 🔒 Jenkins CI Job | 🌐 Logs & Artifacts

CLICK for Detailed Summary

jobs Status
jenkins-ci/mbed-os-ci_cmake-example-GCC_ARM ✔️
jenkins-ci/mbed-os-ci_cmake-example-ARM ✔️
jenkins-ci/mbed-os-ci_unittests ✔️
jenkins-ci/mbed-os-ci_build-greentea-ARM ✔️
jenkins-ci/mbed-os-ci_build-greentea-GCC_ARM ✔️
jenkins-ci/mbed-os-ci_build-example-ARM ✔️
jenkins-ci/mbed-os-ci_build-example-GCC_ARM ✔️
jenkins-ci/mbed-os-ci_build-cloud-example-GCC_ARM ✔️
jenkins-ci/mbed-os-ci_build-cloud-example-ARM ✔️
jenkins-ci/mbed-os-ci_cmake-example-test ✔️
jenkins-ci/mbed-os-ci_greentea-test ✔️

@rajkan01
Copy link
Contributor Author

rajkan01 commented Jan 5, 2021

@0xc0170 This PR is ready for CI, could you start the CI

@0xc0170
Copy link
Contributor

0xc0170 commented Jan 5, 2021

CI restarted

@0xc0170
Copy link
Contributor

0xc0170 commented Jan 5, 2021

@hugueskamba happy with the current state?

@mbed-ci
Copy link

mbed-ci commented Jan 5, 2021

Jenkins CI Test : ✔️ SUCCESS

Build Number: 2 | 🔒 Jenkins CI Job | 🌐 Logs & Artifacts

CLICK for Detailed Summary

jobs Status
jenkins-ci/mbed-os-ci_unittests ✔️
jenkins-ci/mbed-os-ci_cmake-example-ARM ✔️
jenkins-ci/mbed-os-ci_cmake-example-GCC_ARM ✔️
jenkins-ci/mbed-os-ci_build-greentea-ARM ✔️
jenkins-ci/mbed-os-ci_build-greentea-GCC_ARM ✔️
jenkins-ci/mbed-os-ci_build-example-ARM ✔️
jenkins-ci/mbed-os-ci_build-example-GCC_ARM ✔️
jenkins-ci/mbed-os-ci_build-cloud-example-ARM ✔️
jenkins-ci/mbed-os-ci_build-cloud-example-GCC_ARM ✔️
jenkins-ci/mbed-os-ci_cmake-example-test ✔️
jenkins-ci/mbed-os-ci_greentea-test ✔️

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants