Skip to content

Conversation

@ladislas
Copy link
Contributor

@ladislas ladislas commented Sep 13, 2019

Description

The PR updated googletest to v1.8.1. It avoids having a "random" commit used.

Tested with cmake .. and gcc on macOS, but could not test AND with mbed test --unittests but could not complete (see #11485) -- googletest compiles correctly at the beginning though.

The update needs a small change in CMakeLists.txt, based on that: google/googletest#1764 (comment)

Pull request type

[ ] Fix
[ ] Refactor
[ ] Target update
[ ] Functionality change
[ ] Docs update
[x] Test update
[ ] Breaking change

Reviewers

n/a

Release Notes

n/a

@ciarmcom ciarmcom requested a review from a team September 13, 2019 17:00
@ciarmcom
Copy link
Member

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

@0xc0170 0xc0170 requested a review from a team September 16, 2019 07:36
@0xc0170
Copy link
Contributor

0xc0170 commented Sep 16, 2019

The PR updated googletest to v1.8.1. It avoids having a "random" commit used.

👍 for using the version rather than SHA

@ARMmbed/mbed-os-test Please review

@adbridge
Copy link
Contributor

@ARMmbed/mbed-os-test bump - please review (@jamesbeyond fyi)

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.

Could you add a line in the readme:
https://github.com/ARMmbed/mbed-os/blob/master/UNITTESTS/README.md#writing-unit-tests
Saying this is using google tests v1.8.1 ?

@ladislas
Copy link
Contributor Author

@jamesbeyond Just updated the readme, let me know if it works for you!

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.

Looks good! thanks @ladislas

@0xc0170
Copy link
Contributor

0xc0170 commented Oct 1, 2019

CI started

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.

6 participants