Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Agent for Boxcar.io #1323

Merged
merged 5 commits into from Mar 6, 2016
Merged

Agent for Boxcar.io #1323

merged 5 commits into from Mar 6, 2016

Conversation

prathyush
Copy link
Contributor

New agent to send Boxcar notifications to iOS devices.

@cantino
Copy link
Member

cantino commented Feb 26, 2016

Cool @prathyush! Would you be up for adding some RSpec specs to this as well?

@prathyush
Copy link
Contributor Author

@cantino: I haven't written a spec before but I gave a shot at. However, I couldn't quite get to mock/stub HTTParty.post and check if the event is getting generated. Can you take a look at the spec and see if I'm missing anything obvious?

@prathyush
Copy link
Contributor Author

The rspec run failed on Ruby 2.1.5 and Postgresql with the following error:

Failures:

  1) DotHelper with example Agents #agents_dot generates a DOT script
     Failure/Error: expect(agents_dot(@agents)).to match(%r{
       expected "digraph \"Agent Event Flow\"{node[shape=box,style=rounded,target=_blank,fontsize=10];edge[fontsize=10];a1061892863[label=foo];a1061892863->a1061892865[color=\"#999999\"];a1061892863->a1061892864[style=dashed];a1061892864[label=bar1];a1061892865[label=bar2,style=\"rounded,dashed\",color=\"#999999\",fontcolor=\"#999999\"];a1061892865->a1061892866[style=dashed,color=\"#999999\"];a1061892866[label=bar3];}" to match /
                 \A
                 digraph \x20 "Agent \x20 Event \x20 Flow" \{
                   node \[ [^\]]+ \];
                   edge \[ [^\]]+ \];
                   (?<foo>\w+) \[label=foo\];
                   \k<foo> -> (?<bar1>\w+) \[style=dashed\];
                   \k<foo> -> (?<bar2>\w+) \[color="\#999999"\];
                   \k<bar1> \[label=bar1\];
                   \k<bar2> \[label=bar2,style="rounded,dashed",color="\#999999",fontcolor="\#999999"\];
                   \k<bar2> -> (?<bar3>\w+) \[style=dashed,color="\#999999"\];
                   \k<bar3> \[label=bar3\];
                 \}
                 \z
               /x
       Diff:
       @@ -1,16 +1,2 @@
       -/
       -          \A
       -          digraph \x20 "Agent \x20 Event \x20 Flow" \{
       -            node \[ [^\]]+ \];
       -            edge \[ [^\]]+ \];
       -            (?<foo>\w+) \[label=foo\];
       -            \k<foo> -> (?<bar1>\w+) \[style=dashed\];
       -            \k<foo> -> (?<bar2>\w+) \[color="\#999999"\];
       -            \k<bar1> \[label=bar1\];
       -            \k<bar2> \[label=bar2,style="rounded,dashed",color="\#999999",fontcolor="\#999999"\];
       -            \k<bar2> -> (?<bar3>\w+) \[style=dashed,color="\#999999"\];
       -            \k<bar3> \[label=bar3\];
       -          \}
       -          \z
       -        /x
       +"digraph \"Agent Event Flow\"{node[shape=box,style=rounded,target=_blank,fontsize=10];edge[fontsize=10];a1061892863[label=foo];a1061892863->a1061892865[color=\"#999999\"];a1061892863->a1061892864[style=dashed];a1061892864[label=bar1];a1061892865[label=bar2,style=\"rounded,dashed\",color=\"#999999\",fontcolor=\"#999999\"];a1061892865->a1061892866[style=dashed,color=\"#999999\"];a1061892866[label=bar3];}"
     # ./spec/helpers/dot_helper_spec.rb:57:in `block (4 levels) in <top (required)>'

Not sure how to figure out which part of my diff would have caused this issue.

@dsander
Copy link
Collaborator

dsander commented Feb 27, 2016

@prathyush I reran that job, looks like it has just been a random failure.

@prathyush
Copy link
Contributor Author

@dsander Awesome!.

def receive(incoming_events)
incoming_events.each do |event|
payload_interpolated = interpolated(event)
user_credentials = payload_interpolated['user_credentials'] || credential('boxcar_api_key')
Copy link
Member

Choose a reason for hiding this comment

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

Let's just do payload_interpolated['user_credentials'] and then change the instructions to

Please provide your access token in the `user_credentials` option. If you'd like to use a credential, set the `user_credentials` option to `{% credential CREDENTIAL_NAME %}`.

@cantino
Copy link
Member

cantino commented Feb 29, 2016

Left some suggestions @prathyush, thanks!

@prathyush
Copy link
Contributor Author

@cantino Made the changes. Please take a look.

describe "#receive" do
it "sends a message" do
stub(HTTParty).post { {"id" => 1, "message" => "blah", "title" => "blah","source_name" => "Custom Notification"} }
@checker.receive([@event])
Copy link
Member

Choose a reason for hiding this comment

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

Does this spec fail if you break the code? I think you may need mock here so that an expectation is set.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, it fails. I even found a bug (typo) when I first ran this.

@cantino
Copy link
Member

cantino commented Mar 6, 2016

Looks good, thanks for the contribution!

cantino added a commit that referenced this pull request Mar 6, 2016
@cantino cantino merged commit 1faf950 into huginn:master Mar 6, 2016
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.

None yet

3 participants