Skip to content

[daemon] Remove support for appliances#4363

Merged
sharder996 merged 2 commits intomainfrom
remove-appliances
Oct 7, 2025
Merged

[daemon] Remove support for appliances#4363
sharder996 merged 2 commits intomainfrom
remove-appliances

Conversation

@jimporter
Copy link
Copy Markdown
Contributor

@jimporter jimporter commented Sep 16, 2025

This resolves #4120.

@codecov
Copy link
Copy Markdown

codecov Bot commented Sep 16, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 89.39%. Comparing base (011daaf) to head (bda64bd).
⚠️ Report is 909 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #4363      +/-   ##
==========================================
- Coverage   89.70%   89.39%   -0.31%     
==========================================
  Files         260      260              
  Lines       15852    18590    +2738     
==========================================
+ Hits        14219    16617    +2398     
- Misses       1633     1973     +340     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@jimporter jimporter force-pushed the remove-appliances branch 3 times, most recently from 04f8ba5 to 18fdf5f Compare September 16, 2025 21:14
@jimporter jimporter marked this pull request as ready for review September 16, 2025 21:24
@jimporter jimporter force-pushed the remove-appliances branch 5 times, most recently from 1cdae52 to 710462a Compare September 16, 2025 23:22
@jimporter jimporter requested review from ricab and sharder996 and removed request for sharder996 September 17, 2025 00:18
Comment thread tests/test_daemon.cpp
Comment on lines +169 to +178
TEST_F(Daemon, buildsConfig)
{
// Clear our stub image host so that DaemonConfigBuilder::build generates the default.
config_builder.image_hosts.clear();

auto config = config_builder.build();
EXPECT_EQ(config->ssh_username, "ubuntu");
EXPECT_THAT(config->image_hosts, Not(IsEmpty()));
}

Copy link
Copy Markdown
Contributor Author

@jimporter jimporter Sep 17, 2025

Choose a reason for hiding this comment

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

This test is pretty simple and is mainly just to ensure that we can auto-populate image_hosts. Maybe we could add a few more EXPECT_* calls in here to cover the other members...

Copy link
Copy Markdown
Collaborator

@sharder996 sharder996 left a comment

Choose a reason for hiding this comment

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

Would you be able to link some documentation referencing the deprecation of Invoke()?

@jimporter
Copy link
Copy Markdown
Contributor Author

Would you be able to link some documentation referencing the deprecation of Invoke()?

@sharder996 I don't see anything in the actual docs, but this is the warning I get when using Invoke(some_callable) (with some extra newlines added for readability):

/home/jim/cnc/multipass/tests/mock_process_factory.cpp: In constructor
  ‘multipass::test::MockProcess::MockProcess(std::unique_ptr<multipass::ProcessSpec>&&,
    std::vector<multipass::test::MockProcessFactory::ProcessInfo>&)’:
/home/jim/cnc/multipass/tests/mock_process_factory.cpp:43:49:
  warning: ‘typename std::decay<_Tp>::type testing::Invoke(FunctionImpl&&) [with FunctionImpl =
    multipass::test::MockProcess::MockProcess(std::unique_ptr<multipass::ProcessSpec>&&,
    std::vector<multipass::test::MockProcessFactory::ProcessInfo>&)::<lambda()>;
    typename std::decay<_Tp>::type = multipass::test::MockProcess::MockProcess(
      std::unique_ptr<multipass::ProcessSpec>&&,
      std::vector<multipass::test::MockProcessFactory::ProcessInfo>&)::<lambda()>]’
  is deprecated: Actions can now be implicitly constructed from callables.
  No need to create wrapper objects using Invoke(). [-Wdeprecated-declarations]
   43 |     ON_CALL(*this, start()).WillByDefault(Invoke([this] { emit started(); }));
      |                                           ~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~
In file included from /home/jim/cnc/multipass/build/_deps/googletest-src/googlemock/include/gmock/gmock.h:56,
                 from /home/jim/cnc/multipass/tests/common.h:22,
                 from /home/jim/cnc/multipass/tests/mock_process_factory.h:20,
                 from /home/jim/cnc/multipass/tests/mock_process_factory.cpp:18:
/home/jim/cnc/multipass/build/_deps/googletest-src/googlemock/include/gmock/gmock-actions.h:2046:41:
  note: declared here
 2046 | typename std::decay<FunctionImpl>::type Invoke(FunctionImpl&& function_impl) {
      |                                         ^~~~~~

Copy link
Copy Markdown
Collaborator

@sharder996 sharder996 left a comment

Choose a reason for hiding this comment

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

Ok, thanks for the additional details. Just so there's a breadcrumb trail to justify the Invoke() change.

Otherwise, LGTM!

Copy link
Copy Markdown
Contributor

@levkropp levkropp left a comment

Choose a reason for hiding this comment

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

LGTM! The only other references to appliances are related to blueprints tests which are going to be removed shortly as well 😀

@levkropp levkropp enabled auto-merge October 6, 2025 18:10
@sharder996 sharder996 marked this pull request as draft October 7, 2025 01:20
auto-merge was automatically disabled October 7, 2025 01:20

Pull request was converted to draft

@sharder996 sharder996 marked this pull request as ready for review October 7, 2025 01:20
@sharder996 sharder996 added this pull request to the merge queue Oct 7, 2025
Merged via the queue into main with commit c0f5fce Oct 7, 2025
37 of 40 checks passed
@sharder996 sharder996 deleted the remove-appliances branch October 7, 2025 03:52
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.

Remove obsolete appliances

3 participants