[Services] Fix backup test suite build
ClosedPublic
Actions

Authored by • jon on Sep 12 2022, 1:45 PM.

Details

Reviewers

atul
• abosh
varun
• max
• karol

Commits

rCOMMf3f886ab6d54: [Services] Fix backup test suite build

Summary

I would like to add the test suites to the CI, but they have to
build first

Also, a big change is building the testsuite by default, however, this is
the common practice, and generally people opt-out of building tests by doing
-DBUILD_TESTING=OFF when they invoke cmake.

https://linear.app/comm/issue/ENG-1794

Test Plan

nix develop
cd services/backup
rm -rf build && mkdir build && cd build && cmake .. && make -j
ctest # should fail as AWS needs to be configured, but run

# without tests
nix develop
cd services/backup
rm -rf build && mkdir build && cd build && cmake .. -DBUILD_TESTING=OFF && make -j
ctest # should fail to find tests

Diff Detail

Repository

rCOMM Comm

Lint

No Lint Coverage

Unit

No Test Coverage

Event Timeline

• jon created this revision.Sep 12 2022, 1:45 PM

Herald added subscribers: tomek, ashoat. · View Herald TranscriptSep 12 2022, 1:45 PM

Harbormaster returned this revision to the author for changes because remote builds failed.Sep 12 2022, 1:46 PM

Harbormaster failed remote builds in B12092: Diff 16599!

• jon added a parent revision: D5121: [Services] Avoid using GTest in docker.Sep 12 2022, 2:04 PM

Rebase on top of docker fixes

force gtest to be found through config, to void libgtest.a being found

Harbormaster completed remote builds in B12099: Diff 16606.Sep 12 2022, 2:13 PM

Harbormaster completed remote builds in B12103: Diff 16610.Sep 12 2022, 2:19 PM

• jon requested review of this revision.Sep 12 2022, 2:19 PM

Was able to successfully follow Test Plan both with and without tests.

This revision is now accepted and ready to land.Sep 13 2022, 9:16 AM

Closed by commit rCOMMf3f886ab6d54: [Services] Fix backup test suite build (authored by • jon). · Explain WhySep 13 2022, 11:45 AM

This revision was automatically updated to reflect the committed changes.

• jon added a commit: rCOMMf3f886ab6d54: [Services] Fix backup test suite build.

Revision Contents
Changeset List

Path

Size

services/

backup/

CMakeLists.txt

38 lines

Diff 16606

View Options

services/backup/CMakeLists.txt

This file was added.

				div.containerViewerLiked {
				padding: 4px;
				border-radius: 4px;
				background-color: var(--shades-black-80);
				}

				div.container:hover {
				cursor: pointer;
				}
				tomekUnsubmitted Not Done Inline Actions For other tooltip actions we don't need to manually set this style. Can we use the mechanism we already have, or are there any reasons for not doing so? tomek: For other tooltip actions we don't need to manually set this style. Can we use the mechanism we…
				ginsuAuthorUnsubmitted Done Inline Actions I believe the other icons don't need to set this style because it is built into `IcomoonReact`. When I switched the Like action tooltip button with one of the icons the hover style just works out of the box. Wondering if @atul has more context since he is pretty familiar with the icons stuff ginsu: I believe the other icons don't need to set this style because it is built into `IcomoonReact`.
				tomekUnsubmitted Not Done Inline Actions It wouldn't make too much sense for `Icomoon` to provide hover effects for icons because they could be used in other places - not only as buttons. It seems like the style on hover is applied in `message-tooltip.css` div.messageActionButtons svg:hover { cursor: pointer; color: var(--fg); } so it should be enough to apply this style also to the new button. tomek: It wouldn't make too much sense for `Icomoon` to provide hover effects for icons because they…
				ginsuAuthorUnsubmitted Done Inline Actions Ahhhhh that makes sense, thank you for clarifying, I will make the necessary changes ginsu: Ahhhhh that makes sense, thank you for clarifying, I will make the necessary changes

[Services] Fix backup test suite buildClosedPublicActions