Tails

#1 Updated by intrigeri 2014-10-05 06:02:18

• Feature Branch set to test/pidgin-wip

Some very incomplete, non-working and rough bits can be found in the feature branch.

#2 Updated by intrigeri 2014-10-05 06:03:03

• blocked by Feature #7820: Automatically test Pidgin added

#3 Updated by intrigeri 2014-10-05 06:07:29

• blocks deleted (Feature #7820: Automatically test Pidgin)

#4 Updated by intrigeri 2014-10-05 06:08:06

• Assignee set to anonym
• Target version changed from Sustainability_M1 to Tails_1.2.2
• Parent task changed from Feature #6298 to Feature #7820

#5 Updated by intrigeri 2014-10-20 14:06:04

• blocks #8117 added

#6 Updated by anonym 2014-12-12 16:41:52

• Target version changed from Tails_1.2.2 to Tails_1.2.3

#7 Updated by BitingBird 2015-01-07 18:11:23

• Affected tool set to Instant Messaging

#8 Updated by anonym 2015-01-13 14:14:42

• Target version changed from Tails_1.2.3 to Tails_1.3

#9 Updated by anonym 2015-02-24 20:04:08

• Target version changed from Tails_1.3 to Tails_1.3.2

#10 Updated by sajolida 2015-02-26 15:11:17

• Priority changed from Normal to Elevated

I’m concerned to see that this ticket has been postponed for the fourth time. Take into account that this is blocking a funders’ deliverable (#8117) and cannot be postponed anymore this time. So marking this as Elevated.

#11 Updated by Tails 2015-02-27 15:06:17

• Status changed from Confirmed to In Progress

Applied in changeset commit:12b7641009706e1ff3d422fa7a44f5c58fcf2a4d.

#12 Updated by anonym 2015-02-27 15:37:20

• Assignee changed from anonym to kytv
• % Done changed from 0 to 50
• QA Check set to Ready for QA
• Feature Branch changed from test/pidgin-wip to test/8000-8001-pidgin-xmp-otr

#13 Updated by kytv 2015-02-27 23:32:50

• Assignee changed from kytv to anonym
• QA Check changed from Ready for QA to Dev Needed

Looks good and works well. I saw a couple of minor (not functionality breaking) typos which I pushed to my repo.

After the typos are fixed, I say, “merge”. :)

#14 Updated by kytv 2015-02-27 23:55:20

• % Done changed from 50 to 80

Perhaps the new dependencies listed on wiki/src/contribute/release_process/test/usage.mdwn should also be added to the check_dependencies steps in run_test_suite?

#15 Updated by anonym 2015-03-01 20:21:51

• Assignee changed from anonym to intrigeri
• QA Check changed from Dev Needed to Ready for QA

kytv wrote:
> Looks good and works well. I saw a couple of minor (not functionality breaking) typos which I pushed to my repo.
>
> After the typos are fixed, I say, “merge”. :)

Thanks! I’ve merged your branch. Reassigning to intrigeri.

#16 Updated by intrigeri 2015-03-02 18:39:06

• Assignee changed from intrigeri to anonym
• QA Check changed from Ready for QA to Dev Needed

Here’s an initial code review. First of all, congrats! Impressive work, again.

• In: “Then I can receive a response from my friend”, I think we should drop “can”.
• It would be good to minimally document what’s type in def xmpp_account_extract(type). I didn’t understand what it was about before I read related code elsewhere in the test suite.
• Typos in “Clicking the middle, bottom of this immage should querry our” (image, query).
• s/pidgin/Pidgin/ in “Since pidgin set’s the window name to the contact, we have no good”
• I see you removed the “try to create and connect to a new room” test from the manual test suite, while it hasn’t been automated as far as I can see.
• The indentation of the lines that follow otr_key: | is a bit confusing.
• In “be subscribed to each other but no other XMPP account”, s/but/but to/ perhaps?

Now I’m going to try and run these new tests.

#20 Updated by anonym 2015-03-04 21:50:41

• Assignee changed from anonym to intrigeri
• QA Check changed from Dev Needed to Info Needed

intrigeri wrote:
> Here’s an initial code review. First of all, congrats! Impressive work, again.
>
> * In: “Then I can receive a response from my friend”, I think we should drop “can”.

Fixed in commit 09a48d3.

> * It would be good to minimally document what’s type in def xmpp_account_extract(type). I didn’t understand what it was about before I read related code elsewhere in the test suite.

I also changed the name to account since type clearly is a poot choice. What about commit f5f3acc?

> * Typos in “Clicking the middle, bottom of this immage should querry our” (image, query).
> * s/pidgin/Pidgin/ in “Since pidgin set’s the window name to the contact, we have no good”

Fixed in commit adda968, including “set’s”.

> * I see you removed the “try to create and connect to a new room” test from the manual test suite, while it hasn’t been automated as far as I can see.

Wow, I forgot about that. Since the manual test was vague, what do we expect here? What about that we join the room in Pidgin, and then make the bot join the room, and we look for the status message that “someone entered the room” and that’s it? While the bot framework (jabberbot) doesn’t support rooms/MUC, making it join a room is very simple, so the above would be very doable. Something more complicated that involves making it say something there is harder, and making its listen to what’s said in the room (and respond) is even harder.

What do you think?

> * The indentation of the lines that follow otr_key: | is a bit confusing.

How could I improve it? Since the scope is determined by the indentation I don’t think I can do anything about it.

> * In “be subscribed to each other but no other XMPP account”, s/but/but to/ perhaps?

Ok, fixed in commit ce7e09c.

#22 Updated by intrigeri 2015-03-05 06:46:31

• Assignee changed from intrigeri to anonym
• QA Check changed from Info Needed to Dev Needed

#23 Updated by anonym 2015-03-06 00:38:55

• Assignee changed from anonym to intrigeri
• QA Check changed from Dev Needed to Ready for QA

intrigeri wrote:
> >> * I see you removed the “try to create and connect to a new room” test from the manual test suite, while it hasn’t been automated as far as I can see.
>
> > Wow, I forgot about that. Since the manual test was vague, what do we expect here? What about that we join the room in Pidgin, and then make the bot join the room, and we look for the status message that “someone entered the room” and that’s it?
>
> Good enough!

I actually was wrong — jabberbot has support for it. Nice! Hence I also added tests for sending and receiving multi-user chat messages.

> >> * The indentation of the lines that follow otr_key: | is a bit confusing.
>
> > How could I improve it? Since the scope is determined by the indentation I don’t think I can do anything about it.
>
> With | the scope wouldn’t be determined by the indentation, IIRC.
>
> But I take this one back, since you simply copied the (weird) indentation one can find in ~/.purple/otr.private_key.

Ah, you meant the “internal” indentation, not the YAML-scope one. Well, actually it’s libotr* that generates the keys with that indentation, so I just assumed that it’s important.

I think all issues are fixed now, unless I introduced new ones, which might be the case since the multi-user chat test wasn’t so easy to do.

#24 Updated by intrigeri 2015-03-06 08:07:37

• Assignee changed from intrigeri to anonym
• QA Check changed from Ready for QA to Dev Needed

• Thanks for the detailed commit messages, much appreciated.
• Why does focus_pidgin_window have a Pidgin-specific name, and why is it in pidgin.rb at all?
• Maybe we could generated the chatroom name randomly at runtime, to avoid yet adding another local config option?
• Typo in “in the abode example”
• Isn’t it ever needed to wait_and_click for PidginConversationMenuClearScrollback.png instead of assuming that it appears immediately after clicking PidginConversationMenu.png?

#25 Updated by anonym 2015-03-06 12:58:32

• Assignee changed from anonym to intrigeri
• QA Check changed from Dev Needed to Ready for QA
• Feature Branch changed from test/8000-8001-pidgin-xmp-otr to test/8001-8002-pidgin-xmp-otr

intrigeri wrote:
> * Thanks for the detailed commit messages, much appreciated.
> * Why does focus_pidgin_window have a Pidgin-specific name, and why is it in pidgin.rb at all?

Actually, that commit was rewritten — initially I also made it do ... search --pid $PID --name ..., but that actually seemed to make things worse. After rewriting I suppose I didn’t realize this. Hmm. What about VM.focus_xorg_window(window_title)? Done in commit 0fe3f66.

> * Maybe we could generated the chatroom name randomly at runtime, to avoid yet adding another local config option?

I thought about this, but some XMPP services have disabled creating chatrooms for non-admins, like jabber.org. What about making chat_room optional, and using a random string if it’s not set? That’s commits 9d9b375, 5a848d4 and 1c2c654.

Regarding 9d9b375 I’ve added note Feature #8966#note-20 since the code I added in this branch is essentially copy-pasted from that branch, although made prettier by putting it all into functions.

> * Typo in “in the abode example”

Thanks!

> * Isn’t it ever needed to wait_and_click for PidginConversationMenuClearScrollback.png instead of assuming that it appears immediately after clicking PidginConversationMenu.png?

Indeed it is! Great catch! Fixed!

Also, I just noticed that the name of the branch is bad since it’s referencing bug Bug #8000 which it has nothing to do with. I’ve also pushed a branch with the fixed name (but that’s at the same commit) which I’d like you to switch to (for testing + merging) once you’ve reviewed the new commits.

#27 Updated by intrigeri 2015-03-06 19:48:55

• Assignee changed from intrigeri to anonym
• QA Check changed from Ready for QA to Dev Needed

#28 Updated by anonym 2015-03-07 01:20:29

• Assignee changed from anonym to intrigeri
• QA Check changed from Dev Needed to Ready for QA

intrigeri wrote:
> > What about VM.focus_xorg_window(window_title)?
>
> I think VM.focus_window(window_title) would be just enough, no?

Fixed.

#30 Updated by intrigeri 2015-03-09 12:36:38

• blocks Feature #8966: Automatic tests for IRC CTCP in Pidgin added

#31 Updated by intrigeri 2015-03-09 12:39:43

• Assignee changed from intrigeri to anonym
• QA Check changed from Ready for QA to Info Needed

The following test results come from a test suite run from current stable + branches test/7820-pidgin-irc-ctcp and test/8001-8002-pidgin-xmp-otr-intrigeri merged into it.

In “Scenario: Chatting with some friend over XMPP and with OTR”, I see:

    And I start an OTR session with my friend                              # features/step_definitions/pidgin.rb:96
Traceback (most recent call last):
  File "/srv/git/features/scripts/otr-bot.py", line 160, in <module>
    otr_bot.serve_forever()
  File "/usr/lib/python2.7/dist-packages/jabberbot.py", line 728, in serve_forever
    conn.Process(1)
  File "/usr/lib/python2.7/dist-packages/xmpp/dispatcher.py", line 303, in dispatch
    handler['func'](session,stanza)
  File "/usr/lib/python2.7/dist-packages/xmpp/dispatcher.py", line 215, in streamErrorHandler
    raise exc((name,text))
xmpp.protocol.InvalidFrom: (u'invalid-from', '')
    Then Pidgin automatically generates an OTR key                         # features/step_definitions/pidgin.rb:103
      FindFailed: can not find PidginOTRKeyGenPrompt.png on the screen.
      Line ?, in File ? (RuntimeError)
      features/pidgin.feature:52:in `Then Pidgin automatically generates an OTR key'

… note that Cucumber believes that “I start an OTR session with my friend” has succeeded. Anything I can do to help debugging that problem?

#32 Updated by anonym 2015-03-09 14:11:28

• Assignee changed from anonym to intrigeri

intrigeri wrote:
> The following test results come from a test suite run from current stable + branches test/7820-pidgin-irc-ctcp and test/8001-8002-pidgin-xmp-otr-intrigeri merged into it.
>
> In “Scenario: Chatting with some friend over XMPP and with OTR”, I see:
>
> […]
>
> … note that Cucumber believes that “I start an OTR session with my friend” has succeeded.

So, I suppose there’s some error checking missing in the

> Anything I can do to help debugging that problem?

I think the problem the bot has, xmpp.protocol.InvalidFrom: (u'invalid-from', ''), can be explained by this. What XMPP servers are you using? Perhaps you can email you local.yml to me so I can try to reproduce? BTW, do you see that error all the time, or just randomly?

#34 Updated by intrigeri 2015-03-09 15:34:11

• Assignee changed from intrigeri to anonym
• QA Check changed from Info Needed to Dev Needed

#35 Updated by anonym 2015-03-12 17:49:05

• Assignee changed from anonym to intrigeri
• QA Check changed from Dev Needed to Info Needed

I can reproduce the error with xmpp.protocol.InvalidFrom: (u'invalid-from', '') using the account details you sent me. It’s strange. It seems to only happen with some XMPP servers (e.g. im.apinc.org) and when sending the initial request to start an OTR session — other messages are not affected as we can see. Should we block on this?

I’ll send you my local.yml so you can see that it at least works when using two riseup accounts.

#36 Updated by intrigeri 2015-03-15 09:01:28

• Assignee changed from intrigeri to anonym
• QA Check changed from Info Needed to Dev Needed

anonym wrote:
> It seems to only happen with some XMPP servers (e.g. im.apinc.org) and when sending the initial request to start an OTR session — other messages are not affected as we can see. Should we block on this?

I’m not sure we should block on this. If we don’t, it would be good to at least document a list of XMPP servers that are known to work flawlessly with out test suite (and, hopefully, that do not introduce new environment requirements, see Feature #8002#note-32).

> I’ll send you my local.yml so you can see that it at least works when using two riseup accounts.

Followed-up on the XMPP side of things on Feature #8002#note-32.

#37 Updated by intrigeri 2015-03-15 09:12:26

• blocked by deleted (Feature #8966: Automatic tests for IRC CTCP in Pidgin)

#38 Updated by anonym 2015-03-16 15:40:01

• Assignee changed from anonym to intrigeri
• QA Check changed from Dev Needed to Ready for QA

intrigeri wrote:
> anonym wrote:
> > It seems to only happen with some XMPP servers (e.g. im.apinc.org) and when sending the initial request to start an OTR session — other messages are not affected as we can see. Should we block on this?
>
> I’m not sure we should block on this. If we don’t, it would be good to at least document a list of XMPP servers that are known to work flawlessly with out test suite (and, hopefully, that do not introduce new environment requirements, see Feature #8002#note-32).

Done in commit:af57b74.

#39 Updated by anonym 2015-03-16 17:17:09

• blocked by Feature #9062: Migrate otr-bot.py to python-potr added

#40 Updated by intrigeri 2015-03-16 20:36:49

• blocks deleted (Feature #9062: Migrate otr-bot.py to python-potr)

#41 Updated by intrigeri 2015-03-16 20:50:04

• Assignee changed from intrigeri to anonym
• QA Check changed from Ready for QA to Dev Needed

Code review and pidgin.feature both pass at commit:af57b74. Now just blocked by Feature #9062.

#42 Updated by anonym 2015-03-17 11:32:47

• Assignee changed from anonym to intrigeri
• QA Check changed from Dev Needed to Ready for QA

#43 Updated by intrigeri 2015-03-17 12:04:25

• Status changed from In Progress to Fix committed

Applied in changeset commit:ff9063e75d95029c454b763b4d6298dd802ec022.

#44 Updated by intrigeri 2015-03-17 12:12:47

• Assignee deleted (intrigeri)
• QA Check changed from Ready for QA to Pass

#45 Updated by BitingBird 2015-03-22 11:51:05

• Target version changed from Tails_1.3.2 to Tails_1.3.1

#46 Updated by BitingBird 2015-03-23 02:02:19

• Status changed from Fix committed to Resolved