Merge lp:~marcustomlinson/unity-scopes-api/manually_run_scopes into lp:unity-scopes-api/devel

Proposed by Marcus Tomlinson
Status: Merged
Approved by: Michi Henning
Approved revision: 244
Merged at revision: 451
Proposed branch: lp:~marcustomlinson/unity-scopes-api/manually_run_scopes
Merge into: lp:unity-scopes-api/devel
Diff against target: 151 lines (+73/-7)
4 files modified
include/unity/scopes/internal/RegistryObject.h (+1/-0)
src/scopes/internal/RegistryObject.cpp (+7/-7)
test/gtest/scopes/Registry/CMakeLists.txt (+1/-0)
test/gtest/scopes/Registry/Registry_test.cpp (+64/-0)
To merge this branch: bzr merge lp:~marcustomlinson/unity-scopes-api/manually_run_scopes
Reviewer Review Type Date Requested Status
PS Jenkins bot (community) continuous-integration Approve
Michi Henning (community) Approve
Review via email: mp+230049@code.launchpad.net

Commit message

Registry now aware that a scope is running if it was started manually.

To post a comment you must log in.
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Approve (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Approve (continuous-integration)
Revision history for this message
Michi Henning (michihenning) wrote :

Looks reasonable to me, but I think a test would be good.

242. By Marcus Tomlinson

Merged devel

243. By Marcus Tomlinson

Added test for manually started scope to Registry_test

244. By Marcus Tomlinson

Added error handling to manually_started_scope test

Revision history for this message
Marcus Tomlinson (marcustomlinson) wrote :

> Looks reasonable to me, but I think a test would be good.

K, test added

Revision history for this message
Michi Henning (michihenning) wrote :

Sweet, thank you!

review: Approve
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Approve (continuous-integration)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'include/unity/scopes/internal/RegistryObject.h'
2--- include/unity/scopes/internal/RegistryObject.h 2014-07-30 02:25:40 +0000
3+++ include/unity/scopes/internal/RegistryObject.h 2014-08-18 06:51:44 +0000
4@@ -119,6 +119,7 @@
5 mutable std::condition_variable state_change_cond_;
6 core::posix::ChildProcess process_ = core::posix::ChildProcess::invalid();
7 MWPublisher::SPtr reg_publisher_;
8+ bool manually_started_;
9 };
10
11 private:
12
13=== modified file 'src/scopes/internal/RegistryObject.cpp'
14--- src/scopes/internal/RegistryObject.cpp 2014-08-13 19:56:22 +0000
15+++ src/scopes/internal/RegistryObject.cpp 2014-08-18 06:51:44 +0000
16@@ -340,6 +340,7 @@
17 RegistryObject::ScopeProcess::ScopeProcess(ScopeExecData exec_data, MWPublisher::SPtr publisher)
18 : exec_data_(exec_data)
19 , reg_publisher_(publisher)
20+ , manually_started_(false)
21 {
22 }
23
24@@ -452,8 +453,8 @@
25 env["LD_LIBRARY_PATH"] = scope_ld_lib_path; // Overwrite any LD_LIBRARY_PATH entry that may already be there.
26
27 process_ = executor->exec(program, argv, env,
28- core::posix::StandardStream::stdin | core::posix::StandardStream::stdout,
29- exec_data_.confinement_profile);
30+ core::posix::StandardStream::stdin | core::posix::StandardStream::stdout,
31+ exec_data_.confinement_profile);
32 if (process_.pid() <= 0)
33 {
34 clear_handle_unlocked();
35@@ -534,8 +535,7 @@
36 cout << "RegistryObject::ScopeProcess: Process for scope: \"" << exec_data_.scope_id
37 << "\" started manually" << endl;
38
39- // Don't update state_, treat this scope as not running if a locate() is requested
40- return;
41+ manually_started_ = true;
42 }
43 }
44 else if (new_state == Stopped)
45@@ -552,7 +552,7 @@
46 << "\" closed unexpectedly. Either the process crashed or was killed forcefully." << endl;
47 }
48 }
49- else if (new_state == Stopping && state_ != Running)
50+ else if (new_state == Stopping && manually_started_)
51 {
52 if (reg_publisher_)
53 {
54@@ -563,8 +563,8 @@
55 cout << "RegistryObject::ScopeProcess: Manually started process for scope: \""
56 << exec_data_.scope_id << "\" exited" << endl;
57
58- // Don't update state_, treat this scope as not running if a locate() is requested
59- return;
60+ new_state = Stopped;
61+ manually_started_ = false;
62 }
63 state_ = new_state;
64 state_change_cond_.notify_all();
65
66=== modified file 'test/gtest/scopes/Registry/CMakeLists.txt'
67--- test/gtest/scopes/Registry/CMakeLists.txt 2014-07-22 09:54:57 +0000
68+++ test/gtest/scopes/Registry/CMakeLists.txt 2014-08-18 06:51:44 +0000
69@@ -8,6 +8,7 @@
70 add_definitions(-DTEST_RUNTIME_PATH="${CMAKE_CURRENT_BINARY_DIR}")
71 add_definitions(-DTEST_RUNTIME_FILE="${CMAKE_CURRENT_BINARY_DIR}/Runtime.ini")
72 add_definitions(-DTEST_REGISTRY_PATH="${PROJECT_BINARY_DIR}/scoperegistry")
73+add_definitions(-DTEST_SCOPERUNNER_PATH="${PROJECT_BINARY_DIR}/scoperunner")
74 add_definitions(-DTEST_SCOPE_A_PATH="${CMAKE_CURRENT_BINARY_DIR}/scopes/testscopeA")
75
76 add_executable(Registry_test Registry_test.cpp)
77
78=== modified file 'test/gtest/scopes/Registry/Registry_test.cpp'
79--- test/gtest/scopes/Registry/Registry_test.cpp 2014-08-14 10:18:34 +0000
80+++ test/gtest/scopes/Registry/Registry_test.cpp 2014-08-18 06:51:44 +0000
81@@ -302,6 +302,70 @@
82 std::this_thread::sleep_for(std::chrono::milliseconds(500));
83 }
84
85+TEST(Registry, manually_started_scope)
86+{
87+ bool update_received = false;
88+ std::mutex mutex;
89+ std::condition_variable cond;
90+
91+ Runtime::UPtr rt = Runtime::create(TEST_RUNTIME_FILE);
92+ RegistryProxy r = rt->registry();
93+
94+ // Configure testscopeC scope_state_callback
95+ auto conn = r->set_scope_state_callback("testscopeC", [&update_received, &mutex, &cond](bool)
96+ {
97+ std::lock_guard<std::mutex> lock(mutex);
98+ update_received = true;
99+ cond.notify_one();
100+ });
101+
102+ auto wait_for_state_update = [&update_received, &mutex, &cond]
103+ {
104+ // Wait for an update notification
105+ std::unique_lock<std::mutex> lock(mutex);
106+ bool success = cond.wait_for(lock, std::chrono::seconds(5), [&update_received] { return update_received; });
107+ update_received = false;
108+ return success;
109+ };
110+
111+ // Move testscopeC into the scopes folder
112+ filesystem::rename(TEST_RUNTIME_PATH "/other_scopes/testscopeC", TEST_RUNTIME_PATH "/scopes/testscopeC");
113+ std::this_thread::sleep_for(std::chrono::milliseconds(500));
114+
115+ // testscopeC should not be running at this point
116+ EXPECT_FALSE(r->is_scope_running("testscopeC"));
117+
118+ // start testscopeC manually
119+ auto scope_pid = fork();
120+ if (scope_pid == 0)
121+ {
122+ const char* const args[] = {"scoperunner [Registry test]", TEST_RUNTIME_FILE, TEST_RUNTIME_PATH "/scopes/testscopeC/testscopeC.ini", nullptr};
123+
124+ if (execv(TEST_SCOPERUNNER_PATH "/scoperunner", const_cast<char* const*>(args)) < 0)
125+ {
126+ perror("Error starting scoperunner:");
127+ }
128+ exit(0);
129+ }
130+
131+ // testscopeC should now be running
132+ EXPECT_TRUE(wait_for_state_update());
133+ EXPECT_TRUE(r->is_scope_running("testscopeC"));
134+
135+ // stop testscopeC manually
136+ kill(scope_pid, SIGTERM);
137+ int status;
138+ waitpid(scope_pid, &status, 0);
139+
140+ // testscopeC should now be stopped
141+ EXPECT_TRUE(wait_for_state_update());
142+ EXPECT_FALSE(r->is_scope_running("testscopeC"));
143+
144+ // Move testscopeC back into the other_scopes folder
145+ filesystem::rename(TEST_RUNTIME_PATH "/scopes/testscopeC", TEST_RUNTIME_PATH "/other_scopes/testscopeC");
146+ std::this_thread::sleep_for(std::chrono::milliseconds(500));
147+}
148+
149 TEST(Registry, list_update_notify_before_click_folder_exists)
150 {
151 bool update_received = false;

Subscribers

People subscribed via source and target branches

to all changes: