Skip to content

Move events stubs to events dir #14823

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 6 commits into from
Jul 12, 2021
Merged

Move events stubs to events dir #14823

merged 6 commits into from
Jul 12, 2021

Conversation

rajkan01
Copy link
Contributor

@rajkan01 rajkan01 commented Jun 23, 2021 •
edited
Loading

Summary of changes

fixes #14791

  • This PR makes the mbed-stubs-events library more self-contained by removing the dependencies on the monolithic header libraries we created to make implementing the stubs easier. We also move the files to the events directory so the stubs live with the associated component.
  • Add mbed-headers-events library inside CMake of doubles
  • Move events stubs headers from mbed-stubs-headers library to mbed-stubs-events library.
  • Updated connectivity unit test CMake to use mbed-stubs-events-headers as it relies on events stub headers

Impact of changes

None.

Migration actions required

None

Documentation

To be updated

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

Reviewers

@ciarmcom ciarmcom added the release-type: patch Indentifies a PR as containing just a patch label Jun 23, 2021
@ciarmcom ciarmcom requested a review from a team June 23, 2021 16:30
@ciarmcom
Copy link
Member

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

@mergify
Copy link

mergify bot commented Jun 24, 2021

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

@rajkan01 rajkan01 force-pushed the move_events_stub branch 2 times, most recently from 227ae1e to 48153cc Compare June 25, 2021 09:05
@mergify
Copy link

mergify bot commented Jun 30, 2021

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

@rajkan01 rajkan01 force-pushed the move_events_stub branch from 48153cc to e8a3cac Compare July 1, 2021 13:05
@mergify
Copy link

mergify bot commented Jul 1, 2021

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

@rajkan01 rajkan01 force-pushed the move_events_stub branch 2 times, most recently from 2db0f7f to 9334542 Compare July 1, 2021 15:27
LDong-Arm
LDong-Arm suggested changes Jul 5, 2021
View reviewed changes
Copy link
Contributor

@LDong-Arm LDong-Arm left a comment

Choose a reason for hiding this comment

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

Similar to the other unit tests stubs PRs, maybe we can also move mbed-headers-events from UNITTESTS/stubs/CMakeLists.txt into events/tests/UNITTESTS/doubles/CMakeLists.txt?

And files in UNITTESTS/fakes/events/ can be moved/combined into events/tests/UNITTESTS/doubles/ too - fakes are a type of doubles too.

LDong-Arm
LDong-Arm previously requested changes Jul 5, 2021
View reviewed changes
@mergify
Copy link

mergify bot commented Jul 6, 2021

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

@rajkan01 rajkan01 force-pushed the move_events_stub branch from 9334542 to 71d00fa Compare July 8, 2021 09:08
@mergify mergify bot dismissed LDong-Arm’s stale review July 8, 2021 09:11

Pull request has been modified.

@rajkan01
Copy link
Contributor Author

rajkan01 commented Jul 8, 2021

Similar to the other unit tests stubs PRs, maybe we can also move mbed-headers-events from UNITTESTS/stubs/CMakeLists.txt into events/tests/UNITTESTS/doubles/CMakeLists.txt?

And files in UNITTESTS/fakes/events/ can be moved/combined into events/tests/UNITTESTS/doubles/ too - fakes are a type of doubles too.

fakes are a type of doubles but the fakes use the same file name and similar implementation as the original and that implementation differ based on the Device Under Test use case. Now, these fakes are used by ble. Better to keep the fakes as part of the DUT to avoid conflict and duplicate file issue

@rajkan01 rajkan01 requested a review from LDong-Arm July 8, 2021 09:17
@rajkan01 rajkan01 force-pushed the move_events_stub branch from fc5d5f0 to 80ca390 Compare July 8, 2021 15:55
rajkan01 added 5 commits July 9, 2021 02:56
@rajkan01
CMake: Move events stubs sources to the mbed-os/events directory
88410a5 
Move the events stub into the events component directory.
so we can avoid duplicating the mbed-os source tree in a central
UNITTESTS folder.
@rajkan01
CMake: unit-tests: Move mbed-headers-events to the events directory
65d202b 
Move the header-only mbed-headers-events library the unit test stubs
depend on into the events component directory. This makes the events
stubs more self-contained and improves the composition of the library.
@rajkan01
CMake: Move events stub headers to the events/tests/UNITTESTS/double dir
447754f 
Move events stubs headers to the mbed-os/events/tests/UNITTESTS/double
directory and update double CMake to include stub headers
@rajkan01
CMake: unit-tests: Make events stub only depend on headers it uses
652b421 
Previously the events stub library depended on `mbed-headers`, which
is a collection of all available headers in mbed-os. To make it easier
to separate the library, only depend on the headers we're using.
@rajkan01
Remove trailing space
99e16c2
@rajkan01
CMake: Move events fakes into events double dir
863ef79 
Previously events fakes as part of UNITTESTS/fakes, this PR moves
events fakes to events double directory to make events stubs
to be self-contained.
@rajkan01 rajkan01 force-pushed the move_events_stub branch from 80ca390 to 863ef79 Compare July 9, 2021 09:56
LDong-Arm
LDong-Arm approved these changes Jul 12, 2021
View reviewed changes
Copy link
Contributor

@LDong-Arm LDong-Arm left a comment

Choose a reason for hiding this comment

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

LGTM

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

0xc0170 commented Jul 12, 2021

CI started

@mbed-ci
Copy link

mbed-ci commented Jul 12, 2021

Jenkins CI Test : ✔️ SUCCESS

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

CLICK for Detailed Summary

jobs Status
jenkins-ci/mbed-os-ci_unittests ✔️

@0xc0170 0xc0170 merged commit dfd8996 into master Jul 12, 2021
@0xc0170 0xc0170 deleted the move_events_stub branch July 12, 2021 11:51
@mergify mergify bot removed the ready for merge label Jul 12, 2021
@mbedmain mbedmain added release-version: 6.13.0 Release-pending and removed release-type: patch Indentifies a PR as containing just a patch Release-pending labels Jul 14, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@0xc0170 0xc0170 0xc0170 approved these changes

@LDong-Arm LDong-Arm LDong-Arm approved these changes

Assignees
No one assigned
Labels
component: connectivity component: core component: test release-version: 6.13.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Move events stubs to events/tests/UNITTESTS
6 participants
@rajkan01 @ciarmcom @0xc0170 @mbed-ci @LDong-Arm @mbedmain