Skip to content

Drivers directory restructuring #13356

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

Merged
merged 4 commits into from
Aug 5, 2020
Merged

Conversation

gpsimenos
Copy link
Contributor

@gpsimenos gpsimenos commented Jul 27, 2020

Summary of changes

mbed-os/drivers directory restructured according to the internal proposal.

  • Public headers moved into mbed-os/drivers/include/drivers folder
  • Unity tests moved into mbed-os/drivers/tests/UNITTESTS folder
  • Greentea tests moved into mbed-os/drivers/tests/TESTS folder

This is one of a series of PRs aiming to clean up the mbed-os directory structure. The intention is to create a consistent tree among mbed components, following the below structure:

[component name]
├── mbed_lib.json                     // Each component comes with their own config that's appended to the App/System ├── config 
├── CMakeList.txt                     // This depends on the build system choices, part of another PREQ
├── README.md
├── include                           // The top-level include path
│   └── [component name]              // Public headers that can be accessed by the developers - `#include [component name]/*.h`
│       └── internal                  // Internal headers that implement or are included by a public header or are needed by other Mbed OS libraries - `#include [component name]/internal/*.h`
├── source                            // Source files and private headers, available only inside the component's source directory
└── tests
    ├── <framework>                   // <framework> test folder (for example CPPUTEST)
    │   └── <test suite one>          // Can be either source file or a directory
    ├── UNITTESTS                     // Unit tests (previously in UNITTESTS) folder using googletest framework (to be renamed to gtest at a later stage)
    │   └── <unit test suite one>     // Can be either source file or a directory
    └── TESTS                         // Greentea tests (previously in TESTS) folder (to be renamed to greentea at a later stage)
        └── <greentea test suite one> // Can be either source file or a directory
 
mbed-os
└── TESTS (to be renamed to greentea at a later stage)
    ├── integration   
    └── system

Impact of changes

None

Migration actions required

None


Documentation

None


Pull request type

[X] Patch update (Bug fix / Target update / Docs update / Test update / Refactor)
[] 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
mbed test --greentea --compile -c

Build successes:
  * K64F::GCC_ARM::DRIVERS-TESTS-TESTS-MBED_DRIVERS-BUFFERED_SERIAL
  * K64F::GCC_ARM::DRIVERS-TESTS-TESTS-MBED_DRIVERS-CRC
  * K64F::GCC_ARM::DRIVERS-TESTS-TESTS-MBED_DRIVERS-C_STRINGS
  * K64F::GCC_ARM::DRIVERS-TESTS-TESTS-MBED_DRIVERS-DEV_NULL
  * K64F::GCC_ARM::DRIVERS-TESTS-TESTS-MBED_DRIVERS-ECHO
  * K64F::GCC_ARM::DRIVERS-TESTS-TESTS-MBED_DRIVERS-FLASHIAP
  * K64F::GCC_ARM::DRIVERS-TESTS-TESTS-MBED_DRIVERS-GENERIC_TESTS
  * K64F::GCC_ARM::DRIVERS-TESTS-TESTS-MBED_DRIVERS-LP_TICKER
  * K64F::GCC_ARM::DRIVERS-TESTS-TESTS-MBED_DRIVERS-LP_TIMEOUT
  * K64F::GCC_ARM::DRIVERS-TESTS-TESTS-MBED_DRIVERS-LP_TIMER
  * K64F::GCC_ARM::DRIVERS-TESTS-TESTS-MBED_DRIVERS-RACE_TEST
  * K64F::GCC_ARM::DRIVERS-TESTS-TESTS-MBED_DRIVERS-RESET_REASON
  * K64F::GCC_ARM::DRIVERS-TESTS-TESTS-MBED_DRIVERS-RTC
  * K64F::GCC_ARM::DRIVERS-TESTS-TESTS-MBED_DRIVERS-SLEEP_LOCK
  * K64F::GCC_ARM::DRIVERS-TESTS-TESTS-MBED_DRIVERS-STL_FEATURES
  * K64F::GCC_ARM::DRIVERS-TESTS-TESTS-MBED_DRIVERS-TICKER
  * K64F::GCC_ARM::DRIVERS-TESTS-TESTS-MBED_DRIVERS-TIMEOUT
  * K64F::GCC_ARM::DRIVERS-TESTS-TESTS-MBED_DRIVERS-TIMER
  * K64F::GCC_ARM::DRIVERS-TESTS-TESTS-MBED_DRIVERS-TIMEREVENT
  * K64F::GCC_ARM::DRIVERS-TESTS-TESTS-MBED_DRIVERS-UNBUFFERED_SERIAL
  * K64F::GCC_ARM::DRIVERS-TESTS-TESTS-MBED_DRIVERS-WATCHDOG
  * K64F::GCC_ARM::DRIVERS-TESTS-TESTS-MBED_DRIVERS-WATCHDOG_RESET

===================================================================

mbed test --greentea --compile -c  --app-config TESTS/configs/baremetal.json -DMBED_EXTENDED_TESTS

Build successes:
  * K64F::GCC_ARM::DRIVERS-TESTS-TESTS-MBED_DRIVERS-BUFFERED_SERIAL
  * K64F::GCC_ARM::DRIVERS-TESTS-TESTS-MBED_DRIVERS-CRC
  * K64F::GCC_ARM::DRIVERS-TESTS-TESTS-MBED_DRIVERS-C_STRINGS
  * K64F::GCC_ARM::DRIVERS-TESTS-TESTS-MBED_DRIVERS-DEV_NULL
  * K64F::GCC_ARM::DRIVERS-TESTS-TESTS-MBED_DRIVERS-ECHO
  * K64F::GCC_ARM::DRIVERS-TESTS-TESTS-MBED_DRIVERS-FLASHIAP
  * K64F::GCC_ARM::DRIVERS-TESTS-TESTS-MBED_DRIVERS-GENERIC_TESTS
  * K64F::GCC_ARM::DRIVERS-TESTS-TESTS-MBED_DRIVERS-LP_TICKER
  * K64F::GCC_ARM::DRIVERS-TESTS-TESTS-MBED_DRIVERS-LP_TIMEOUT
  * K64F::GCC_ARM::DRIVERS-TESTS-TESTS-MBED_DRIVERS-LP_TIMER
  * K64F::GCC_ARM::DRIVERS-TESTS-TESTS-MBED_DRIVERS-RACE_TEST
  * K64F::GCC_ARM::DRIVERS-TESTS-TESTS-MBED_DRIVERS-RESET_REASON
  * K64F::GCC_ARM::DRIVERS-TESTS-TESTS-MBED_DRIVERS-RTC
  * K64F::GCC_ARM::DRIVERS-TESTS-TESTS-MBED_DRIVERS-SLEEP_LOCK
  * K64F::GCC_ARM::DRIVERS-TESTS-TESTS-MBED_DRIVERS-STL_FEATURES
  * K64F::GCC_ARM::DRIVERS-TESTS-TESTS-MBED_DRIVERS-TICKER
  * K64F::GCC_ARM::DRIVERS-TESTS-TESTS-MBED_DRIVERS-TIMEOUT
  * K64F::GCC_ARM::DRIVERS-TESTS-TESTS-MBED_DRIVERS-TIMER
  * K64F::GCC_ARM::DRIVERS-TESTS-TESTS-MBED_DRIVERS-TIMEREVENT
  * K64F::GCC_ARM::DRIVERS-TESTS-TESTS-MBED_DRIVERS-UNBUFFERED_SERIAL
  * K64F::GCC_ARM::DRIVERS-TESTS-TESTS-MBED_DRIVERS-WATCHDOG
  * K64F::GCC_ARM::DRIVERS-TESTS-TESTS-MBED_DRIVERS-WATCHDOG_RESET

===================================================================

mbed test --unittests --compile -c

. . .
[ 11%] Built target drivers-tests-UNITTESTS-Watchdog.MbedOS
. . .
[ 32%] Built target drivers-tests-UNITTESTS-PwmOut.MbedOS
. . .

Unit tests built successfully.

Reviewers

@evedon
@ashok-rao
@LDong-Arm


@ciarmcom ciarmcom added the release-type: patch Indentifies a PR as containing just a patch label Jul 27, 2020
@ciarmcom ciarmcom requested review from ashok-rao, evedon, LDong-Arm and a team July 27, 2020 11:00
@ciarmcom
Copy link
Member

@gpsimenos, thank you for your changes.
@evedon @LDong-Arm @ashok-rao @ARMmbed/mbed-os-core @ARMmbed/mbed-os-maintainers please review.

evedon
evedon previously approved these changes Jul 27, 2020
@mergify mergify bot added needs: CI and removed needs: review labels Jul 27, 2020
@gpsimenos gpsimenos force-pushed the gp-drivers-restruct branch from 110d660 to 81f4be7 Compare July 28, 2020 08:20
@gpsimenos
Copy link
Contributor Author

Rebased.

@mergify mergify bot dismissed evedon’s stale review July 28, 2020 08:20

Pull request has been modified.

@0xc0170
Copy link
Contributor

0xc0170 commented Jul 28, 2020

CI restarted

@mbed-ci
Copy link

mbed-ci commented Jul 28, 2020

Test run: FAILED

Summary: 1 of 6 test jobs failed
Build number : 1
Build artifacts

Failed test jobs:

  • jenkins-ci/mbed-os-ci_greentea-test

@mergify mergify bot added needs: work and removed needs: CI labels Jul 28, 2020
@0xc0170
Copy link
Contributor

0xc0170 commented Jul 28, 2020

@gpsimenos There are lot of failures for multiple targets, do you have any of them to retest?

@gpsimenos
Copy link
Contributor Author

@gpsimenos There are lot of failures for multiple targets, do you have any of them to retest?

I'm looking into it. From the failing targets, I have the K64F and DISCO_L475VG_IOT01A.

@gpsimenos
Copy link
Contributor Author

The greentea tests that are failing are those that have a corresponding host test. It seems the host test scripts are not found due to the restructuring. I'm working on a fix.

@0xc0170
Copy link
Contributor

0xc0170 commented Jul 28, 2020

This should fix it #13362 ? Or more to come?

@gpsimenos
Copy link
Contributor Author

Yes that will do it for rtos, and we'll need a similar change in this PR and #13298.

@mergify
Copy link

mergify bot commented Jul 28, 2020

This PR cannot be merged due to conflicts. Please rebase to resolve them.

Copy link
Contributor

@jamesbeyond jamesbeyond left a comment

Choose a reason for hiding this comment

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

@gpsimenos @0xc0170

Hi, my PR #13362 only fixed the issue caused by #13264

In this PR you'll need to do some similar fix
copy mbed-os/TESTS/host_tests/timing_drift_auto.py to mbed-os/drivers/tests/TESTS/host_tests/timing_drift_auto.py
because this file is required by

Do NOT move it, because it still need by :

CI not cover this timing drift test, because CI can't guarantee the time accuracy. It so got missed.
For the rest of failure. similarly, you'll need to go though all the greentea tests you moved, to identify what host_test python code they use, and move/copy them over accordingly.

@gpsimenos
Copy link
Contributor Author

@gpsimenos @0xc0170

Hi, my PR #13362 only fixed the issue caused by #13264

In this PR you'll need to do some similar fix
copy mbed-os/TESTS/host_tests/timing_drift_auto.py to mbed-os/drivers/tests/TESTS/host_tests/timing_drift_auto.py
because this file is required by

Do NOT move it, because it still need by :

CI not cover this timing drift test, because CI can't guarantee the time accuracy. It so got missed.
For the rest of failure. similarly, you'll need to go though all the greentea tests you moved, to identify what host_test python code they use, and move/copy them over accordingly.

Thanks @jamesbeyond I was thinking if there's a better way to avoid duplicating the host tests?

@gpsimenos
Copy link
Contributor Author

@jamesbeyond Could we change this script to make it look in the root TESTS directory for the host tests, regardless of the location of the device binary? https://github.com/ARMmbed/mbed-os-tools/blob/5a14958aa49eb5764afba8e1dc3208cae2955cd7/packages/mbed-greentea/mbed_greentea/mbed_test_api.py#L110

@mergify mergify bot dismissed jamesbeyond’s stale review July 31, 2020 08:46

Pull request has been modified.

@gpsimenos gpsimenos force-pushed the gp-drivers-restruct branch from b444615 to bb5c2cf Compare July 31, 2020 09:05
@gpsimenos
Copy link
Contributor Author

Rebased. Discussed with @jamesbeyond and agreed to duplicate some host tests in drivers test directory as a temporary solution, until the new mbed tools allow a better solution. CI should now pass.

@gpsimenos
Copy link
Contributor Author

Ping @0xc0170 @adbridge

@adbridge
Copy link
Contributor

adbridge commented Aug 4, 2020

CI started

@mergify mergify bot added needs: CI and removed needs: work labels Aug 4, 2020
@mbed-ci
Copy link

mbed-ci commented Aug 4, 2020

Test run: SUCCESS

Summary: 6 of 6 test jobs passed
Build number : 2
Build artifacts

@adbridge adbridge merged commit 7418b9d into ARMmbed:master Aug 5, 2020
@jeromecoutant
Copy link
Collaborator

Minor comment:
This makes test name ( ex: drivers-tests-tests-mbed_drivers-echo) not very nice...

@LDong-Arm
Copy link
Contributor

LDong-Arm commented Aug 11, 2020

Minor comment:
This makes test name ( ex: drivers-tests-tests-mbed_drivers-echo) not very nice...

It's a limitation of the build system we'll resolve in the future. For example, the second "tests" comes from the TESTS/ directory which is a magic keyword to identify Greentea tests. Also "mbed_drivers" is a redundant part we can't remove for now.

It'll look more concise once we have improved the build system in the future.

@mbedmain mbedmain added release-version: 6.2.1 Release-pending and removed release-type: patch Indentifies a PR as containing just a patch Release-pending labels Aug 16, 2020
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.

10 participants