Feat: Add GTest/GMock support for ESP32 #66
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Removed static from base path functions to clear warnings. If this function is in a header it is public and should not be static.
Removed unused code
|
@ciband Thanks for submitting this pull request, a maintainer will get back to you shortly! |
|
@faustbrian @air1one - please review this in the next few days. Be sure to explicitly select labels so I know what's going on. If no reviewer appears after a week, a reminder will be sent out. |
ghost
added
Status: Needs Review
|
@ciband Your pull request doesn't follow our contribution guidelines. Please review and correct it. |
Let the project config handle it.
Since ESP32 is now supported by GTest, re-enable it. Disable ESP8266 until it can be supported.
|
@ciband The ci/circleci: build-linux-default job is failing as of 17c4de641710e88a599a43e17da1fdea8154b4f0. Please review the logs for more information. Once you've pushed the fixes, the build will automatically re-run. Thanks! |
|
Once google/googletest#2098 is merged, CI should completed and this PR will be good to go. |
|
googletest PR has been merged and PIO appears to be updated but the actual downloaded library from PIO is still including the source file that is causing the duplicate symbols which cause the failing build. Will continue to investigate. |
|
There was an issue with the googletest PR. This PR fixes it. Waiting on it to be merged. |
|
PR has been merged and PIO library is updated but the build still failed like the PIO library was not updated. Will keep monitoring. |
|
PIO library registry is updated and CI passes now. This PR should be good to go. |
faustbrian
previously approved these changes
Feb 13, 2019
|
@ciband A contributor has approved this PR. A maintainer will merge this PR shortly. If it shouldn't be merged yet, please leave a comment saying so and we'll wait. Thank you for your contribution! |
ghost
added
the
Status: Contributor Approved
Closed
spkjp
previously approved these changes
Feb 13, 2019
|
macOS:
|
|
Not sure what is going on there. PIO build should be the same on Windows,Mac,Linux. Works on Windows and Linux (via CI). Do you have the latest PIO and platform updates installed? |
|
You can try nuking the .pio* directories in the test directory and rebuild. Looks like something with the ESP32 platform is not happy. |
|
@ciband However, ESP32 loops tests with: void setup() {
Serial.begin(115200);
while (!Serial); // for the Arduino Leonardo/Micro only
delay(100);
testing::InitGoogleMock();
delay(1000);
}
void loop() {
RUN_ALL_TESTS();
}Changing to the following made tests run once on-board: void setup() {
Serial.begin(115200);
while (!Serial); // for the Arduino Leonardo/Micro only
delay(100);
testing::InitGoogleMock();
delay(1000);
RUN_ALL_TESTS();
}
void loop() { }Can you confirm this? |
|
Yes it does. If we want it to do a one-shit then we can do that. I will push a change.
|
…/cpp-client into feat/add_platformio_gmock
|
Thank you, @ciband !! |
faustbrian
approved these changes
Feb 14, 2019
|
@ciband A contributor has approved this PR. A maintainer will merge this PR shortly. If it shouldn't be merged yet, please leave a comment saying so and we'll wait. Thank you for your contribution! |
|
@ciband Your pull request has been merged but was not assigned a bounty tier. @air1one @faustbrian - please assign a bounty tier to this pull request in the next few days. |
ghost
removed
the
Status: Needs Review
faustbrian
added
the
Bounty: Tier 4
|
@ciband Your pull request has been merged and marked as tier 3. It will earn you $25 USD. |
ghost
mentioned this pull request
boldninja
added
Bounty: Tier 3
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Proposed changes
Converts the IoT tests to use newly supported googletest and removes AUnit usage. This re-enables the unit test builds for ESP32.
Types of changes
Checklist
Further comments
This PR only supports ESP32, which is better than the current support of nothing. The next step is to support ESP8266. This is a work in progress.
googletest was not supported on our IoT platforms so I added support in googletest. Ark has the distinction of being the first project to use this new feature.
The following are all the commits required to make this happen:
Helps close #27