Update all existing hook repos with the latest changes in this template

[honzajavorek]

This template was updated with some minor changes recently. We should update the local copies in each hooks repo to contain those changes.

• Ruby
• Python - Update Aruba test suite to contain the latest changes dredd-hooks-python#28
• PHP
• Go
• Perl

Related to: #9, #10, #12, #13, #14

[gonzalo-bulnes]

Hi @honzajavorek,

While updating the Ruby hooks I figured out that I was missing an entire feature file. (I had removed it when stabilizing the dependencies versions quite some time ago as part of apiaryio/dredd-hooks-ruby#16 - my bad.) That's why it didn't fail when I updated Aruba. : /

This timeout seems to be a known Aruba issue, or in fact a sort of lack of feature (the step I wait for output to contain "Starting" was not meant to be used in an interactive context - source).

I can make it work partially (read unreliably) using Aruba v0.6.2, however. (Which is probably why it didn't pose problems when the feature was originally written.)

Possible workaround

Find a reasonable amount of time to wait:

# features/tcp_server.feature

#...

  Background:
    Given I run `dredd-hooks-ruby` interactively
    And I run `sleep 2` # replaces the step that waited for output

  Scenario: TCP server
    When I connect to the server
    # ...

Remaining concerns

• The whole feature sometimes hangs towards the end and doesn't finish or timeout with Aruba v0.6.2... ?? Aruba v0.14.2 seems to do fine.
• I did remove the part of the features helper that killed the handler - after trying unsuccessfully your workaround and figuring out accidentally that killing the handler was not needed. I'm not sure that's okay though... or to which extent it may affect the tests independence.

Here is a proof of concept, and the corresponding build.

\cc @ddelnano @w-vi

[honzajavorek]

Interesting! So Aruba "officially" doesn't have a way how to wait for some output of a running process. That may explain why the tests were sometimes flaky (not remembering whether specifically this one though). In that case, I think we're left with sleeping, indeed. I agree we could go that way.

As for killing the handler, I'd prefer to replace the bash lines with something more robust. It may not be necessary to kill the handler for running the test suite in CI, but I'm not sure I could locally successfully run the suite repeatedly if there are processes left. Isn't there a Ruby gem to provide a way how to easily kill processes? That could even bring us closer to portability of the test suite (support for Windows is coming soon).

[gonzalo-bulnes]

Hi @honzajavorek,

I can't find a way to cleanly kill processes by name. There are a few Ruby wrappers to manage processes, but they seem to be intended to be managed by PID (at the *nix level) - we can use such a wrapper, but I'm not sure it really qualifies as more robust. Any thoughts @ddelnano?

[honzajavorek]

I'm just experimenting with ps-node in the Dredd test suite to solve the same thing. There are situations where processes are killed by name, and I'm trying to use ps's lookup first to get PID and then kill. I'm not sure it really works on all platforms yet though and I'm really not sure I'd like to mix Ruby and Node like that. I think the lib parses ps output (or win equivalent to ps command output).

Similar Ruby solution would be the best. I can look into it later, but first I need to verify that it actually works at least in Node, in the tests.

Or, maybe, we're solving a wrong problem here and the test can be done in a different way completely, but I'm really short of ideas how to ensure a good tear down. Stale processes can lead to "port taken" problems when locally running the same tests again, and similar issues.

[ddelnano]

@gonzalo-bulnes @honzajavorek about the issue about when the hooks server is still listening on the port. Not sure if this is useful but someone asked me to merge functionality on the php hooks side that could be useful although its not going to solve the final tear down.

The php hooks accept a --force flag which essentially stops a php process listening on the port the hooks server is about to connect to. Now the check is still based off of parsing the output of a bash command (which I now just realized is *nix specific :( ) but wondering if allowing the hooks server to always connect might solve some / most of the pain?

The implementation can be found here

[ungrim97]

I have updated the perl one as part of ungrim97/Dredd-Hooks#7

[honzajavorek]

@ddelnano I'm not sure it would be a good idea to do that always. Killing a process in someone's system seems quite invasive to me. If a user chooses to do so by using a special option (--force), then it's their wish and consent, but I wouldn't introduce this as a default behavior.

[honzajavorek]

Just to reiterate, the current changes in the template's master are problematic. We wanted to test any changes on Ruby hooks before we put them into the template, to have a proof everything works, but unfortunately, Ruby hooks missed a file, so the proof wasn't complete.

Before introducing the current changes to other hooks, we need to get the template fixed, i.e. get the template working first with either Ruby ( apiaryio/dredd-hooks-ruby#31 ) or Python ( apiaryio/dredd-hooks-python#28 ) hooks.

The only missing part seems to be to find a nice way how to kill processes in Ruby, preferably by a dedicated library (and if dreaming is allowed, also cross-platform library).

I've tried to find something and inspired by some forum posts, I got as far as this:

$ gem install sys-proctable

require 'sys/proctable'

Sys::ProcTable.ps.each { |ps|
  puts ps.cmdline

  # Ideally, there would be something like... (ruby-like pseudocode)
  #
  # if ps.cmdline contains 'hook-handler...'
  #   Process.kill('KILL', ps.pid)
  # end
}

@gonzalo-bulnes Do you think this would solve the issue?

[gonzalo-bulnes]

I'll also make a quick list of what we know so far to :P

About killing the dredd-hooks process after the tests finish

• The issue being primarily the potential lack of potability of the shell scripts we can come up with, I think that something like sys-proctable brings the benefit of moving that problem out to a dedicated project.

• Why a Ruby solution? Mainly because Ruby is already available in all the projects that use this template. (It's required by Cucumber.) Going with Ruby allows us not to make further assumptions (is bash available or is it another shell, etc.)

On the other hand...

It may not be necessary to kill the handler for running the test suite in CI, but I'm not sure I could locally successfully run the suite repeatedly if there are processes left. (From comment above - bold highlighting is mine -- GB)

I cannot find a Dredd Hooks process left behind after the Dredd Hooks Ruby test suite ran on my machine.

I've augmented a little bit the script from apiaryio/dredd-hooks-ruby#31 with debug output:

# features/support/env.rb

require 'aruba/cucumber'
require "sinatra/base"

Before do
  puts "Killing server..."
  system "echo 'Searching for hooks:'"
  system "ps aux | grep 'dredd-hooks'"
  system "echo 'Searching for server.rb:'"
  system "ps aux | grep 'server.rb'"
  system "for i in `ps axu | grep 'server.rb' | grep ruby | awk '{print $2}'`; do kill -9 $i; done > /dev/null 2>&1"
  sleep 3

  @aruba_timeout_seconds = 10
end

And I see no process to be killed at any point... neither when greping for "hooks" nor "server.rb".

Sample output:

# Output of: cucumber features/tcp_server.feature

Feature: TCP server and messages

Searching for hooks:
gonzalobulnes     8871   0.0  0.0  2434840    800 s010  S+   11:07am   0:00.00 grep dredd-hooks
gonzalobulnes     8869   0.0  0.0  2435436   1100 s010  S+   11:07am   0:00.00 sh -c ps aux | grep 'dredd-hooks'
Searching for server.rb:
gonzalobulnes     8875   0.0  0.0  2434840    800 s010  S+   11:07am   0:00.00 grep server.rb
gonzalobulnes     8873   0.0  0.0  2435436   1100 s010  S+   11:07am   0:00.00 sh -c ps aux | grep 'server.rb'
  Background:                                    # features/tcp_server.feature:3
      Killing server...
    Given I run `dredd-hooks-ruby` interactively # aruba-0.14.2/lib/aruba/cucumber/command.rb:49
    And I run `sleep 2`                          # aruba-0.14.2/lib/aruba/cucumber/command.rb:13

  Scenario: TCP server                                       # features/tcp_server.feature:7
    Then it should start listening on localhost port "61321" # features/step_definitions/dredd_steps.rb:13

Searching for hooks:
gonzalobulnes     8888   0.6  0.0  2435436   1100 s010  S+   11:07am   0:00.00 sh -c ps aux | grep 'dredd-hooks'
gonzalobulnes     8890   0.4  0.0  2434840    800 s010  S+   11:07am   0:00.00 grep dredd-hooks
Searching for server.rb:
gonzalobulnes     8894   0.0  0.0  2434840    800 s010  S+   11:07am   0:00.00 grep server.rb
gonzalobulnes     8892   0.0  0.0  2435436   1100 s010  S+   11:07am   0:00.00 sh -c ps aux | grep 'server.rb'
  Scenario: Message exchange for event beforeEach                       # features/tcp_server.feature:10
      Killing server...
    When I connect to the server                                        # features/step_definitions/dredd_steps.rb:18
    And I send a JSON message to the socket:                            # features/step_definitions/dredd_steps.rb:22
      """
      {"event": "beforeEach", "uuid": "1234-abcd", "data": {"key":"value"}}
      """
    And I send a newline character as a message delimiter to the socket # features/step_definitions/dredd_steps.rb:27
    Then I should receive same response                                 # features/step_definitions/dredd_steps.rb:31
    And I should be able to gracefully disconnect                       # features/step_definitions/dredd_steps.rb:40

# [...] (There is nothing significantly different in the other scenarios.)

Hypothesis

So I'm left wondering: is it possible that Aruba takes care of stopping the processes it runs interactively?

I found this clue: https://www.relishapp.com/cucumber/aruba/v/0-9-0/docs/commands/interactive-process-control#all-processes-are-stopped-before-checking-for-filesystem-changes Sadly, I was unable to find the issue that's referred there for context.
The Relishapp documentation doesn't exist for Aruba 0.14.2, but the feature is still in the docs anyway.

We're not performing filesystem checks, but I still see no left-over processes.

Could it be that interactive processes are executed in a sub-shell (hence not visible to ps aux from another shell) and Aruba clean that sub-shell once done (which would explain why I can run the test suite as many times as I want without ever killing dredd-hooks-ruby)?

I'm sorry, these are more questions than answers. But I'm now more keen on removing the killer script altogether than replicating them in Ruby. If we're not killing anything anyway :S

Any thoughts?
Could someone confirm that they can run the features multiple times even with the killer scripts disabled?

[honzajavorek]

The thing is, I think we need to confirm the tests clean up even if they're testing something which doesn't work as expected. That means, if Dredd won't correctly stop its child processes, the test suite should take care of any leftovers. In case Dredd works as expected and cleans up itself, it's possible these tear-downs do not catch and kill anything and that would be all right.

If we can rely on Aruba automatically cleaning up any possible leftover child processes or subprocesses (if we experiment and we're still not sure, we can ask in their issues), then I'm completely okay with removing the custom killing logic altogether 👍