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

[tests] Split out cli client tests #2348

Draft
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

townsend2010
Copy link
Collaborator

This is a first step to help organize the client cli tests and make them more manageable.

@codecov
Copy link

codecov bot commented Nov 18, 2021

Codecov Report

Merging #2348 (9980d57) into main (862f44f) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##             main    #2348   +/-   ##
=======================================
  Coverage   84.83%   84.83%           
=======================================
  Files         205      205           
  Lines       10216    10216           
=======================================
  Hits         8667     8667           
  Misses       1549     1549           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 862f44f...9980d57. Read the comment docs.

Copy link
Collaborator

@Saviq Saviq left a comment

Choose a reason for hiding this comment

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

This needs a rebase, and a s/2021/2022/ in the headers :)

Noice otherwise :)

@Saviq
Copy link
Collaborator

Saviq commented Jan 28, 2022

@townsend2010 bump?

@townsend2010
Copy link
Collaborator Author

Due to the changes I made for the authentication stuff, a MockPlatform needs to be defined before
StrictMock<MockDaemonRpc> mock_daemon in ClientTestFixture so the set_server_socket_restrictions() call in theDaemonRpc ctor can be mocked. However, due to how the singleton boiler plate is currently done, this MockPlatform definition cannot be done in a header due to the following error:

In file included from /home/townsend/multipass/multipass/tests/client/test_cli_client.cpp:18:
/home/townsend/multipass/multipass/tests/client/client_test_fixture.h:81:8: error: ‘multipass::test::ClientTestFixture’ has a field ‘multipass::test::ClientTestFixture::platform_attr’ whose type uses the anonymous namespace [-Werror=subobject-linkage]
   81 | struct ClientTestFixture : public Test
      |        ^~~~~~~~~~~~~~~~~

One of the main points of this refactoring is to move the MockDaemonRpc object into a class (ClientTestFixture) in a shared header and removing this defeats the purpose of most of this refactoring. Until this issue can be overcome, I think this should wait.

@townsend2010 townsend2010 marked this pull request as draft January 28, 2022 16:21
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.

None yet

2 participants