Skip to content
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

Added GMock to UT project compilation #1902

Merged
merged 1 commit into from
Oct 19, 2022

Conversation

mbronk
Copy link
Contributor

@mbronk mbronk commented Oct 16, 2022

Rationale: Allows to use full portfolio of matchers/assertions.
E.g. http://google.github.io/googletest/reference/matchers.html#container-matchers

Codebase is included with GTest (=was already there, just not compiled)

auto expected = std::vector<uint8_t>({
0xAC, 0xF5, 0x00, 0x24, 0x02, 0x00, 0x00, 0x00, 0x00, 0xAC, 0xD6, 0x01});
auto actual = ac.getRaw();
EXPECT_THAT(std::vector<uint8_t>(actual, actual + kArgoBits / 8),
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is mostly to prove out that CI links w/ GMock correctly.
The benefit in using ::ElementsAreArray over EXPECT_STATE_EQ is marginal in this case (the only material difference is GMock's matcher would spit out all differences, and not stop on first error).


The wrapping into vector is there, mostly to include readability (I don't believe the extra copy for UT hurts anything).
In theory this could be written like so:

  EXPECT_THAT(*reinterpret_cast<uint8_t(*)[kArgoBits / 8]>(ac.getRaw()),
              ::testing::ElementsAreArray(expected));

Anyways, this is mostly a sample to prove integration works. As long as the i-face is a C-Style array, use of fancy matchers doesn't bring much benefit (but they start to shine once STL containers get used)

test/Makefile Outdated Show resolved Hide resolved
$(GTEST_DIR)/src/gtest_main.cc

gmock-all.o : $(GMOCK_SRCS_)
$(CXX) $(CPPFLAGS) -I$(GTEST_DIR) -I$(GMOCK_DIR) $(CXXFLAGS) \
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think it is possible to use wildcard rules here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Absolutely, though these "GMock" Makefile receipes are fairly generic and written this way.
See https://github.com/ceph/gmock/blob/master/make/Makefile for example

@mbronk
Copy link
Contributor Author

mbronk commented Oct 16, 2022

Hi,
This PR is really to test the waters :)
I'm currently adding support for new type of Argo AC remote and figured I could use some of GMock's goodness in the UTs I'm adding there (I personally believe GTest+GMock are much more powerful when used together).

Since it was just lying there... I added GMock to compilation (at the moment it is not doing anything useful, the only "Argo" file I changed was to prove out CI can compile and link with it w/o issues).
I can also revert that sample change if you like (though IMHO it is adding 0.01% of value still).

@crankyoldgit - are you OK with this direction?

Signed-off-by: Mateusz Bronk [email protected]
@mbronk mbronk force-pushed the private/mbronk/added_gmock branch from bfd9034 to f1d35b8 Compare October 16, 2022 16:53
@crankyoldgit
Copy link
Owner

Hi, This PR is really to test the waters :) I'm currently adding support for new type of Argo AC remote and figured I could use some of GMock's goodness in the UTs I'm adding there (I personally believe GTest+GMock are much more powerful when used together).

Since it was just lying there... I added GMock to compilation (at the moment it is not doing anything useful, the only "Argo" file I changed was to prove out CI can compile and link with it w/o issues). I can also revert that sample change if you like (though IMHO it is adding 0.01% of value still).

@crankyoldgit - are you OK with this direction?

Yep. I'm fine with it.

Copy link
Owner

@crankyoldgit crankyoldgit left a comment

Choose a reason for hiding this comment

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

LGTM!

@crankyoldgit crankyoldgit merged commit e8234cc into crankyoldgit:master Oct 19, 2022
crankyoldgit added a commit that referenced this pull request Mar 5, 2023
_v2.8.5 (20230305)_

**[Bug Fixes]**
- Missing argument in use of midea function (#1959 #1958)
- IRMQTTServer: Improve HA MQTT climate handling. (#1911)
- SEND_SANYO_AC88: Fix poor cut-n-paste error (#1905 #1897)

**[Features]**
- IRMQTTServer: SHT-3x Temperature Sensor Support (#1951)
- IRMQTTServer: HA multi output discovery (#1947)
- IRMQTTServer: extended with new A/C common fields (#1940)
- IRMQTTServer: Sync the on state to power from mode for HA (#1946)
- Experimental basic support for Carrier 84-bit protocol. (#1945 #1943)
- Add support the WowWee 11-Bit RoboRaptor-X protocol. (#1939 #1938)
- Added 'sensorTemperature' and 'iFeel' to IRac (common) (#1928)
- Added extra 'mid' option for Fan & SwingV to IRac (#1929)
- Added "commandType" to IRAc (#1921)
- Added support for Argo WREM-3 A/C remote protocol [part1] (#1920)
- Added Dutch (nl-NL) translation (#1907)
- ARGO: Improve code & add support for decoding 32bit sensor msgs. (#1906 #1859)
- Added support for Gorenje cooker hood IR protocol (#1888 #1887)

**[Misc]**
- Update `XMP` status to Stable (#1944)
- upgrade to a later version of `googletest` (#1936)
- MITSUBISHI128: Added model to supported protocol (#1924)
- Added Dutch (nl-NL) README (#1908)
- Added GMock to UT Makefile (#1902)
- Update HA example config for HA 2022.6+ (#1901 #1900)
crankyoldgit added a commit that referenced this pull request May 8, 2023
_v2.8.5 (20230508)_

**[Bug Fixes]**
- Fix a bug where we never detached the timer interrupt on ESP32s. (#1984 #1983)
- Missing argument in use of midea function (#1959 #1958)
- IRMQTTServer: Improve HA MQTT climate handling. (#1911)
- SEND_SANYO_AC88: Fix poor cut-n-paste error (#1905 #1897)

**[Features]**
- Add support for a 40bit variant of the standard Panasonic protocol (#1977 @1976)
- Initial support for York AC protocol (#1889)
- IRMQTTServer: SHT-3x Temperature Sensor Support (#1951)
- IRMQTTServer: HA multi output discovery (#1947)
- IRMQTTServer: extended with new A/C common fields (#1940)
- IRMQTTServer: Sync the on state to power from mode for HA (#1946)
- Experimental basic support for Carrier 84-bit protocol. (#1945 #1943)
- Add support the WowWee 11-Bit RoboRaptor-X protocol. (#1939 #1938)
- Added 'sensorTemperature' and 'iFeel' to IRac (common) (#1928)
- Added extra 'mid' option for Fan & SwingV to IRac (#1929)
- Added "commandType" to IRAc (#1921)
- Added support for Argo WREM-3 A/C remote protocol [part1] (#1920)
- Added Dutch (nl-NL) translation (#1907)
- ARGO: Improve code & add support for decoding 32bit sensor msgs. (#1906 #1859)
- Added support for Gorenje cooker hood IR protocol (#1888 #1887)

**[Misc]**
- Add Electrolux YKR-H/531E as a supported device (#1981 #1980)
- Update `XMP` status to Stable (#1944)
- upgrade to a later version of `googletest` (#1936)
- MITSUBISHI128: Added model to supported protocol (#1924)
- Added Dutch (nl-NL) README (#1908)
- Added GMock to UT Makefile (#1902)
- Update HA example config for HA 2022.6+ (#1901 #1900)
- Add a `d1_mini_noMDNS` build option to `IRMQTTServer`. (#1985)
@crankyoldgit crankyoldgit mentioned this pull request May 8, 2023
crankyoldgit added a commit that referenced this pull request May 8, 2023
## _v2.8.5 (20230508)_

**[Bug Fixes]**
- Fix a bug where we never detached the timer interrupt on ESP32s. (#1984 #1983)
- Missing argument in use of midea function (#1959 #1958)
- IRMQTTServer: Improve HA MQTT climate handling. (#1911)
- SEND_SANYO_AC88: Fix poor cut-n-paste error (#1905 #1897)

**[Features]**
- Add support for a 40bit variant of the standard Panasonic protocol (#1977 @1976)
- Initial support for York AC protocol (#1889)
- IRMQTTServer: SHT-3x Temperature Sensor Support (#1951)
- IRMQTTServer: HA multi output discovery (#1947)
- IRMQTTServer: extended with new A/C common fields (#1940)
- IRMQTTServer: Sync the on state to power from mode for HA (#1946)
- Experimental basic support for Carrier 84-bit protocol. (#1945 #1943)
- Add support the WowWee 11-Bit RoboRaptor-X protocol. (#1939 #1938)
- Added 'sensorTemperature' and 'iFeel' to IRac (common) (#1928)
- Added extra 'mid' option for Fan & SwingV to IRac (#1929)
- Added "commandType" to IRAc (#1921)
- Added support for Argo WREM-3 A/C remote protocol [part1] (#1920)
- Added Dutch (nl-NL) translation (#1907)
- ARGO: Improve code & add support for decoding 32bit sensor msgs. (#1906 #1859)
- Added support for Gorenje cooker hood IR protocol (#1888 #1887)

**[Misc]**
- Add Electrolux YKR-H/531E as a supported device (#1981 #1980)
- Update `XMP` status to Stable (#1944)
- upgrade to a later version of `googletest` (#1936)
- MITSUBISHI128: Added model to supported protocol (#1924)
- Added Dutch (nl-NL) README (#1908)
- Added GMock to UT Makefile (#1902)
- Update HA example config for HA 2022.6+ (#1901 #1900)
- Add a `d1_mini_noMDNS` build option to `IRMQTTServer`. (#1985)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants