New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 863794 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Aug 30
Cc:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug


Participants' hotlists:
ML-Service


Sign in to add a comment

ML Service: Add a basic test of the Chromium client library

Project Member Reported by amoylan@chromium.org, Jul 16

Issue description

We need to at least have a simple unit test that chromeos::machine_learning::ServiceConnection::BindModelProvider works (i.e. returns). It can call the Fake version of the D-Bus client.

At present, the code is unused and so not built, and actually has a typo introduced late in the CL review process that prevents build...

 
Project Member

Comment 1 by bugdroid1@chromium.org, Jul 26

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/453eba239f74a28e5046925a04c2378673b762fa

commit 453eba239f74a28e5046925a04c2378673b762fa
Author: Andrew Moylan <amoylan@chromium.org>
Date: Thu Jul 26 16:23:54 2018

Add basic ServiceConnection unit test

The is just testing that ServiceConnection::BindModelProvider returns
successfully. The Mojo invitation won't go anywhere beyond the no-op
FakeMachineLearningClient D-Bus client.

To make this test compile, I had to fix an #include from an
overlooked file rename in an earlier CL ...

Bug:  863794 

Change-Id: Ifd31fea950145561cd5954ae16e930b801bfe49b
Reviewed-on: https://chromium-review.googlesource.com/1139942
Commit-Queue: Andrew Moylan <amoylan@chromium.org>
Reviewed-by: Ken Rockot <rockot@chromium.org>
Reviewed-by: Dan Erat <derat@chromium.org>
Cr-Commit-Position: refs/heads/master@{#578329}
[modify] https://crrev.com/453eba239f74a28e5046925a04c2378673b762fa/chromeos/dbus/fake_machine_learning_client.cc
[modify] https://crrev.com/453eba239f74a28e5046925a04c2378673b762fa/chromeos/services/BUILD.gn
[modify] https://crrev.com/453eba239f74a28e5046925a04c2378673b762fa/chromeos/services/machine_learning/DEPS
[modify] https://crrev.com/453eba239f74a28e5046925a04c2378673b762fa/chromeos/services/machine_learning/public/cpp/BUILD.gn
[modify] https://crrev.com/453eba239f74a28e5046925a04c2378673b762fa/chromeos/services/machine_learning/public/cpp/service_connection.cc
[modify] https://crrev.com/453eba239f74a28e5046925a04c2378673b762fa/chromeos/services/machine_learning/public/cpp/service_connection.h
[add] https://crrev.com/453eba239f74a28e5046925a04c2378673b762fa/chromeos/services/machine_learning/public/cpp/service_connection_unittest.cc

Project Member

Comment 2 by bugdroid1@chromium.org, Jul 27

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/dfa5e05a6dad878d247f9ca884c7793427f4d53f

commit dfa5e05a6dad878d247f9ca884c7793427f4d53f
Author: Stephen McGruer <smcgruer@chromium.org>
Date: Fri Jul 27 14:29:19 2018

Revert "Add basic ServiceConnection unit test"

This reverts commit 453eba239f74a28e5046925a04c2378673b762fa.

Reason for revert: Failing Linux ASAN/LSAN bot: https://logs.chromium.org/logs/chromium/buildbucket/cr-buildbucket.appspot.com/8939853423518476256/+/steps/chromeos_unittests/0/logs/ServiceConnectionTest.BindModelProvider/0

Original change's description:
> Add basic ServiceConnection unit test
> 
> The is just testing that ServiceConnection::BindModelProvider returns
> successfully. The Mojo invitation won't go anywhere beyond the no-op
> FakeMachineLearningClient D-Bus client.
> 
> To make this test compile, I had to fix an #include from an
> overlooked file rename in an earlier CL ...
> 
> Bug:  863794 
> 
> Change-Id: Ifd31fea950145561cd5954ae16e930b801bfe49b
> Reviewed-on: https://chromium-review.googlesource.com/1139942
> Commit-Queue: Andrew Moylan <amoylan@chromium.org>
> Reviewed-by: Ken Rockot <rockot@chromium.org>
> Reviewed-by: Dan Erat <derat@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#578329}

TBR=derat@chromium.org,rockot@chromium.org,amoylan@chromium.org

Change-Id: I9878dee8885c78f88c0af50e70e921861ab87663
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  863794 
Reviewed-on: https://chromium-review.googlesource.com/1152371
Reviewed-by: Stephen McGruer <smcgruer@chromium.org>
Commit-Queue: Stephen McGruer <smcgruer@chromium.org>
Cr-Commit-Position: refs/heads/master@{#578632}
[modify] https://crrev.com/dfa5e05a6dad878d247f9ca884c7793427f4d53f/chromeos/dbus/fake_machine_learning_client.cc
[modify] https://crrev.com/dfa5e05a6dad878d247f9ca884c7793427f4d53f/chromeos/services/BUILD.gn
[modify] https://crrev.com/dfa5e05a6dad878d247f9ca884c7793427f4d53f/chromeos/services/machine_learning/DEPS
[modify] https://crrev.com/dfa5e05a6dad878d247f9ca884c7793427f4d53f/chromeos/services/machine_learning/public/cpp/BUILD.gn
[modify] https://crrev.com/dfa5e05a6dad878d247f9ca884c7793427f4d53f/chromeos/services/machine_learning/public/cpp/service_connection.cc
[modify] https://crrev.com/dfa5e05a6dad878d247f9ca884c7793427f4d53f/chromeos/services/machine_learning/public/cpp/service_connection.h
[delete] https://crrev.com/1568d039155f062ae4427d4506ba98fb304268c0/chromeos/services/machine_learning/public/cpp/service_connection_unittest.cc

Project Member

Comment 3 by bugdroid1@chromium.org, Jul 30

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/be88eebcf1e4c1659e1cc52961213715713dfc98

commit be88eebcf1e4c1659e1cc52961213715713dfc98
Author: Andrew Moylan <amoylan@chromium.org>
Date: Mon Jul 30 23:38:16 2018

Reland "Add basic ServiceConnection unit test"

This is a reland of 453eba239f74a28e5046925a04c2378673b762fa

Removed the call to mojo::core::Init from the test startup. It
caused a memory leak because //chromeos/run_all_unittests.cc
already calls it.

Original change's description:
> Add basic ServiceConnection unit test
>
> The is just testing that ServiceConnection::BindModelProvider returns
> successfully. The Mojo invitation won't go anywhere beyond the no-op
> FakeMachineLearningClient D-Bus client.
>
> To make this test compile, I had to fix an #include from an
> overlooked file rename in an earlier CL ...
>
> Bug:  863794 
>
> Change-Id: Ifd31fea950145561cd5954ae16e930b801bfe49b
> Reviewed-on: https://chromium-review.googlesource.com/1139942
> Commit-Queue: Andrew Moylan <amoylan@chromium.org>
> Reviewed-by: Ken Rockot <rockot@chromium.org>
> Reviewed-by: Dan Erat <derat@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#578329}

Tested by: manual detect_leaks=1 invocation of the test.

TBR=rockot@chromium.org,derat@chromium.org

Bug:  863794 
Change-Id: I1258de74273551cebdc9dd1a76f86a35ffa50147
Reviewed-on: https://chromium-review.googlesource.com/1154647
Reviewed-by: Andrew Moylan <amoylan@chromium.org>
Commit-Queue: Andrew Moylan <amoylan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#579234}
[modify] https://crrev.com/be88eebcf1e4c1659e1cc52961213715713dfc98/chromeos/dbus/fake_machine_learning_client.cc
[modify] https://crrev.com/be88eebcf1e4c1659e1cc52961213715713dfc98/chromeos/services/BUILD.gn
[modify] https://crrev.com/be88eebcf1e4c1659e1cc52961213715713dfc98/chromeos/services/machine_learning/DEPS
[modify] https://crrev.com/be88eebcf1e4c1659e1cc52961213715713dfc98/chromeos/services/machine_learning/public/cpp/BUILD.gn
[modify] https://crrev.com/be88eebcf1e4c1659e1cc52961213715713dfc98/chromeos/services/machine_learning/public/cpp/service_connection.cc
[modify] https://crrev.com/be88eebcf1e4c1659e1cc52961213715713dfc98/chromeos/services/machine_learning/public/cpp/service_connection.h
[add] https://crrev.com/be88eebcf1e4c1659e1cc52961213715713dfc98/chromeos/services/machine_learning/public/cpp/service_connection_unittest.cc

Status: Fixed (was: Started)

Sign in to add a comment