Support dynamic external_tool_url. Fixes #21

[atodorov]

@jhilden please review.

[jhilden]

hmm. I would think there must be a better way. It should be possible to pass in the lambda as a config option just like you do in a regular app.

[atodorov]

Well I tried but after Rails initializes the middleware it freezes the array. I tried deleting or swapping the current instance of I18nViz middleware with another one, which has the lambda configured but without luck. Maybe it is due to Rails specifics, I don't know, but this is the only way I managed to get this test working.

[atodorov]

I've updated the test to take into account the currently selected locale. You can either use parameters from the environment or I18n.locale (.default_locale). I've also added a sample translation file for Bulgarian, for completeness.

[atodorov]

@jhilden ping. Please see my previous comment about the monkey patch in the test. If we had rspec (and rspec-mocks) I could make this code look a bit cleaner by doing something like

allow(I18nViz::Middleware).to receive(:external_tool_url).and_return( lambda {} )

which essentially does the same as the monkey-patch behind the scenes. What do you think, which one would you prefer ?

[jhilden]

@atodorov I'm sorry that I didn't have time to take another bigger look at your code, it's certainly on my list.

concerning Rspec: I'm definitely not against switching to Rspec at all, I also prefer it nowadays, but I currently just don't have the time to do it.

[atodorov]

@jhilden ping. any update here, it's been a while. Are you still against the monkey-patch piece of code within the tests ?

[jhilden]

thanks for pinging me again @atodorov I totally forgot takeing another look at this.

I'm not strictly against the monkey patch within the tests, IF there is no easy other solution. So that would not be a blocker for me to merge the PR.

However, the tests seem to be failing now with:

I18nVizDynamicUrlTest#test_dynamic_external_url_with_Bulgarian_locale:
Selenium::WebDriver::Error::WebDriverError:  Unable to find Mozilla geckodriver. Please download the server from https://github.com/mozilla/geckodriver/releases and place it somewhere on your PATH. More info at https://developer.mozilla.org/en-US/docs/Mozilla/QA/Marionette/WebDriver.

I think this should be worked on.

[atodorov]

@jhilden the error looks like Travis wasn't able to install all the required gems although the log appears to show everything as installed before we run the tests. I'm pretty sure it is not related to the current changes but I can't help further ATM.

[atodorov]

@jhilden I've spent more time on this today. The selenium problem (cherry-picked here) and the HTML nested tags are fixed in #25 (I hope).

Now about the failures we're observing here:

  1) Failure:
I18nVizDynamicUrlTest#test_dynamic_external_url_with_Bulgarian_locale [/home/senko/i18n_viz/test/i18n_viz_test.rb:104]:
Expected false to be truthy.

  2) Failure:
I18nVizDynamicUrlTest#test_dynamic_external_url_with_English_locale [/home/senko/i18n_viz/test/i18n_viz_test.rb:98]:
Expected false to be truthy.

The assertion which fails is looking for the tooltip links when i18n_viz is enabled so I fixed it with

    assert page.has_css?(".i18n-viz")
    first('.i18n-viz').hover