Paths

Table of Contentst

-
services/tunnelbroker/
-
tunnelbroker/
2/4
CMakeLists.txt

[Services] Use BUILD_TESTING for tunnelbroker build
ClosedPublic
Actions

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

Details

Reviewers

atul
• abosh
varun
• max
• karol

Commits

rCOMM54348ade8842: [Services] Use BUILD_TESTING for tunnelbroker build

Summary

The biggest 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/tunnelbroker
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/tunnelbroker
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:47 PM

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

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

Harbormaster failed remote builds in B12093: Diff 16600!

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

• jon added a child revision: D5119: [Chore] Cleanup tunnelbroker CMakeLists.txt.

Rebase on top of docker fixes

Harbormaster completed remote builds in B12100: Diff 16607.Sep 12 2022, 2:16 PM

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

In this case:

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

the tests aren't running (and failing). Instead, I'm getting the following (which I'd expect to get--and do--with -DBUILD_TESTING=OFF):

This revision now requires changes to proceed.Sep 13 2022, 9:27 AM

Default BUILD_TESTING to ON

the tests aren't running (and failing).

this was my bad, forgot to flip the default to ON for this service

https://phab.comm.dev/D5118?vs=16607&id=16624#toc

Sweet, looks good!

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

Harbormaster failed remote builds in B12116: Diff 16624!Sep 13 2022, 9:54 AM

(Looks like build failure was intermittent BuildKite issue, kicked it off again)

Harbormaster completed remote builds in B12116: Diff 16624.Sep 13 2022, 9:57 AM

Closed by commit rCOMM54348ade8842: [Services] Use BUILD_TESTING for tunnelbroker 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: rCOMM54348ade8842: [Services] Use BUILD_TESTING for tunnelbroker build.

Revision Contents
Changeset List

Path

Size

services/

tunnelbroker/

CMakeLists.txt

7 lines

Diff 16607

View Options

services/tunnelbroker/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] Use BUILD_TESTING for tunnelbroker buildClosedPublicActions