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

BICBridgeGRPCService Testing Methods Development #33

Draft
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

among5
Copy link
Contributor

@among5 among5 commented Mar 1, 2023

Add in mock IImplant and IImplantFactory as well as tests for BICBridgeGRPCService

@among5 among5 requested a review from jeffherron March 1, 2023 08:33
// Finally assemble the server.
gRPCServer = builder.BuildAndStart();
std::cout << "Server listening on " << server_address << std::endl;

Copy link
Member

Choose a reason for hiding this comment

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

let's remove extra whitespace


// Check that when no bridges are connected, listBridges can be called without any error
EXPECT_EQ(val.ok(), true);

Copy link
Member

Choose a reason for hiding this comment

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

We should check that no bridges were returned?

Copy link
Member

Choose a reason for hiding this comment

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

need to check both that the grpc call succeed (which is what you're doing (val.ok is true) but also that the reply is what you expect.

@among5 among5 requested a review from jeffherron March 13, 2023 23:40
Copy link
Member

@jeffherron jeffherron left a comment

Choose a reason for hiding this comment

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

more comments


// Per-test teardown
// Called after each test
void TearDown() override {
Copy link
Member

Choose a reason for hiding this comment

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

should we tear down the device/bridge/info services? I think so? Everything put together in setup should be freed in my opinion.

}

// Resources shared by all tests.
static cortec::implantapi::CExternalUnitInfo* unitInfo1;
Copy link
Member

Choose a reason for hiding this comment

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

can we make these public variables and refer to them in tests as BICBridgeGRPCServiceTests.uintInfo1 (etc)

// Mock of a IImplantFactory object
MockIImplantFactory* BICBridgeGRPCServiceTest::mockManager = nullptr;

TEST_F(BICBridgeGRPCServiceTest, ListBridgesWithoutConnections) {
Copy link
Member

Choose a reason for hiding this comment

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

We may(tbd?) need to update this to be TEST_F(BICBridgeGRPCServiceTest theTestEnv, ...) to refer to the testenv to refer to the (now) public variables e.g. unitInfo1.

int myFunct(int x, int y)
{
return x+y;
}


// Check that when one bridge is connected, scanBridges can be called without any error
EXPECT_EQ(val.ok(), true);

Copy link
Member

Choose a reason for hiding this comment

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

Expect eq(reply->bridge_size(), 1) I think


// Check that when no bridges are connected, describeBridge can be called without any error
EXPECT_EQ(val.ok(), true);

Copy link
Member

Choose a reason for hiding this comment

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

should check that list is zero length? some other status check?

@jeffherron jeffherron self-requested a review October 2, 2023 20:30
@jeffherron jeffherron marked this pull request as draft October 23, 2023 17:22
@jeffherron jeffherron changed the title Add in BICBridgeGRPCService Tests BICBridgeGRPCService Testing Methods Development Oct 23, 2023
@jeffherron
Copy link
Member

Moved back to draft status after student graduated without addressing final comments.

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.

2 participants