Merge lp:~pedronis/ubuntu-push/fixes-into-vivid into lp:ubuntu-push

Proposed by Samuele Pedroni
Status: Rejected
Rejected by: Samuele Pedroni
Proposed branch: lp:~pedronis/ubuntu-push/fixes-into-vivid
Merge into: lp:ubuntu-push
Diff against target: 128 lines (+50/-7)
3 files modified
messaging/messaging.go (+8/-3)
messaging/messaging_test.go (+40/-2)
ubuntu-push-client.go (+2/-2)
To merge this branch: bzr merge lp:~pedronis/ubuntu-push/fixes-into-vivid
Reviewer Review Type Date Requested Status
John Lenton (community) Approve
Review via email: mp+259575@code.launchpad.net

Commit message

* skip over Gone messaging notifications to address lp:1451510, eagerly identify them in Tags, tests
* truncate runtime.Stack result to size, otherwise we log \x00s or old stuff

Description of the change

bring in recent fixes into vivid overlay

To post a comment you must log in.
Revision history for this message
John Lenton (chipaca) wrote :

Nice!

review: Approve
Revision history for this message
Samuele Pedroni (pedronis) wrote :

seems not to fix the problem enough (as tested by QA), just forget landing for now, don't revert code though because we think it's not worse/wrong

Unmerged revisions

148. By Samuele Pedroni

bring in recent fixes

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'messaging/messaging.go'
2--- messaging/messaging.go 2015-01-22 09:52:07 +0000
3+++ messaging/messaging.go 2015-05-20 08:09:52 +0000
4@@ -81,6 +81,10 @@
5 func (mmu *MessagingMenu) cleanUpNotifications() {
6 mmu.lock.Lock()
7 defer mmu.lock.Unlock()
8+ mmu.doCleanUpNotifications()
9+}
10+
11+func (mmu *MessagingMenu) doCleanUpNotifications() {
12 for nid, payload := range mmu.notifications {
13 if payload.Gone {
14 // sweep
15@@ -122,10 +126,11 @@
16 func (mmu *MessagingMenu) Tags(app *click.AppId) []string {
17 orig := app.Original()
18 tags := []string(nil)
19- mmu.lock.RLock()
20- defer mmu.lock.RUnlock()
21+ mmu.lock.Lock()
22+ defer mmu.lock.Unlock()
23+ mmu.doCleanUpNotifications()
24 for _, payload := range mmu.notifications {
25- if payload.App.Original() == orig {
26+ if payload.App.Original() == orig && !payload.Gone {
27 tags = append(tags, payload.Tag)
28 }
29 }
30
31=== modified file 'messaging/messaging_test.go'
32--- messaging/messaging_test.go 2014-11-25 18:33:24 +0000
33+++ messaging/messaging_test.go 2015-05-20 08:09:52 +0000
34@@ -48,8 +48,6 @@
35 cRemoveNotification = func(a, n string) {
36 ms.log.Debugf("REMOVE: app: %s, not: %s", a, n)
37 }
38- // just in case
39- cNotificationExists = nil
40 }
41
42 func (ms *MessagingSuite) TearDownSuite(c *C) {
43@@ -61,6 +59,8 @@
44 func (ms *MessagingSuite) SetUpTest(c *C) {
45 ms.log = helpers.NewTestLogger(c, "debug")
46 ms.app = clickhelp.MustParseAppId("com.example.test_test_0")
47+ // just in case
48+ cNotificationExists = nil
49 }
50
51 func (ms *MessagingSuite) TestPresentPresents(c *C) {
52@@ -129,6 +129,13 @@
53 }
54
55 func (ms *MessagingSuite) TestTagsListsTags(c *C) {
56+ existsCount := 0
57+ // patch cnotificationexists to return true
58+ cNotificationExists = func(did string, nid string) bool {
59+ existsCount++
60+ return true
61+ }
62+
63 mmu := New(ms.log)
64 f := func(s string) *launch_helper.Notification {
65 card := launch_helper.Card{Summary: "tag: \"" + s + "\"", Persist: true}
66@@ -138,8 +145,14 @@
67 c.Check(mmu.Tags(ms.app), IsNil)
68 c.Assert(mmu.Present(ms.app, "notif1", f("one")), Equals, true)
69 ms.checkTags(c, mmu.Tags(ms.app), []string{"one"})
70+ c.Check(existsCount, Equals, 1)
71+ existsCount = 0
72+
73 c.Assert(mmu.Present(ms.app, "notif2", f("")), Equals, true)
74 ms.checkTags(c, mmu.Tags(ms.app), []string{"one", ""})
75+ c.Check(existsCount, Equals, 2)
76+ existsCount = 0
77+
78 // and an empty notification doesn't count
79 c.Assert(mmu.Present(ms.app, "notif3", &launch_helper.Notification{Tag: "X"}), Equals, false)
80 ms.checkTags(c, mmu.Tags(ms.app), []string{"one", ""})
81@@ -150,7 +163,32 @@
82 c.Check(mmu.Tags(ms.app), IsNil)
83 }
84
85+func (ms *MessagingSuite) TestTagsSkipGone(c *C) {
86+ existsCount := 0
87+ // patch cnotificationexists to return false
88+ cNotificationExists = func(did string, nid string) bool {
89+ existsCount++
90+ return false
91+ }
92+
93+ mmu := New(ms.log)
94+ f := func(s string) *launch_helper.Notification {
95+ card := launch_helper.Card{Summary: "tag: \"" + s + "\"", Persist: true}
96+ return &launch_helper.Notification{Card: &card, Tag: s}
97+ }
98+
99+ c.Check(mmu.Tags(ms.app), IsNil)
100+ c.Assert(mmu.Present(ms.app, "notif1", f("one")), Equals, true)
101+ ms.checkTags(c, mmu.Tags(ms.app), []string(nil))
102+ c.Check(existsCount, Equals, 1)
103+}
104+
105 func (ms *MessagingSuite) TestClearClears(c *C) {
106+ // patch cnotificationexists to return true
107+ cNotificationExists = func(did string, nid string) bool {
108+ return true
109+ }
110+
111 app1 := ms.app
112 app2 := clickhelp.MustParseAppId("com.example.test_test-2_0")
113 app3 := clickhelp.MustParseAppId("com.example.test_test-3_0")
114
115=== modified file 'ubuntu-push-client.go'
116--- ubuntu-push-client.go 2015-01-21 21:31:39 +0000
117+++ ubuntu-push-client.go 2015-05-20 08:09:52 +0000
118@@ -35,8 +35,8 @@
119 buf := make([]byte, 1<<20)
120 for {
121 <-sigs
122- runtime.Stack(buf, true)
123- log.Printf("=== received SIGQUIT ===\n*** goroutine dump...\n%s\n*** end\n", buf)
124+ sz := runtime.Stack(buf, true)
125+ log.Printf("=== received SIGQUIT ===\n*** goroutine dump...\n%s\n*** end", buf[:sz])
126 }
127 }()
128 }

Subscribers

People subscribed via source and target branches