Skip to content

Conversation

@yperess
Copy link
Contributor

@yperess yperess commented Sep 6, 2022

  1. Remove the module tests (they're no longer needed since upstream FFF runs better CI now)
  2. Update the header to match the pending PR at Support custom function signatures meekrosoft/fff#113

@zephyrbot
Copy link

The following west manifest projects have been modified in this Pull Request:

Name Old Revision New Revision Diff
fff zephyrproject-rtos/fff@6ce5ba2 (zephyr) N/A N/A

Note: This message is automatically posted and updated by the Manifest GitHub Action.

@yperess
Copy link
Contributor Author

yperess commented Sep 6, 2022

Failed compliance check is a false positive, the () cannot be moved closed to RETURN otherwise the macro expansion will treat RETURN as a macro function

@yperess yperess force-pushed the peress/fff-tests branch 2 times, most recently from fee3747 to aa02bf5 Compare September 6, 2022 18:45
fabiobaltieri
fabiobaltieri previously approved these changes Sep 6, 2022
aaronemassey
aaronemassey previously approved these changes Sep 8, 2022
@yperess
Copy link
Contributor Author

yperess commented Sep 8, 2022

@carlescufi what do you think of this? The CI on fff has been updated to use github actions so we also will be running those on our fork (once that PR merges) but we have a chicken & egg issue that I can't merge those changes without rolling back the additional commit that allowed us to run the fff tests on our CI. What I'd like to do:

  1. Merge this commit to remove FFF from the CI of this repo
  2. Roll back Add support for Zephyr fff#1
  3. Pull all the upstream changes from https://github.com/meekrosoft/fff
  4. Update the CI to just run a diff between the fff.h in our fork vs this repo (they should always match)

@carlescufi
Copy link
Member

@carlescufi what do you think of this? The CI on fff has been updated to use github actions so we also will be running those on our fork (once that PR merges) but we have a chicken & egg issue that I can't merge those changes without rolling back the additional commit that allowed us to run the fff tests on our CI. What I'd like to do:

  1. Merge this commit to remove FFF from the CI of this repo
  2. Roll back Add support for Zephyr fff#1
  3. Pull all the upstream changes from https://github.com/meekrosoft/fff
  4. Update the CI to just run a diff between the fff.h in our fork vs this repo (they should always match)

I have no objections with the proposed plan, but I'd like @stephanosio to weigh in on it as well.

@stephanosio stephanosio added this to the v3.3.0 milestone Sep 19, 2022
@stephanosio
Copy link
Member

As long as the "roll back" and the "pull" are done in a non-destructive way (i.e. no force push), no objections. It is important that the commit(s) that are referenced by previous Zephyr releases are valid.

@yperess
Copy link
Contributor Author

yperess commented Sep 19, 2022

As long as the "roll back" and the "pull" are done in a non-destructive way (i.e. no force push), no objections. It is important that the commit(s) that are referenced by previous Zephyr releases are valid.

Yup, that's exactly what I had in mind, the PR is ready to go at https://github.com/zephyrproject-rtos/fff/pull/2/commits. You can see there that the first commit is a revert then a fetch from upstream.

fabiobaltieri
fabiobaltieri previously approved these changes Oct 3, 2022
Yuval Peress added 2 commits October 3, 2022 09:58
I've update FFF to run tests in the native CI for GitHub, so we no
longer need to run these extra tests. Remove the fff module from
west.yml since the module was only used in the CI and the header
was directly included in the Zephyr tree.

See zephyrproject-rtos/fff#2

Signed-off-by: Yuval Peress <[email protected]>
Update fff.h to the pending PR upstream which allows for using a custom
function signature. This enables the use of C++ std::function as the
mock.

Signed-off-by: Yuval Peress <[email protected]>
@nashif nashif merged commit a7979f8 into zephyrproject-rtos:main Oct 4, 2022
@yperess yperess deleted the peress/fff-tests branch December 6, 2022 17:08
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.

7 participants