Redmine

BTW, the removal of include ActionDispatch::TestProcess is unrelated cleanup, should not be in the patch, sorry for that.

Same for database_cleaner no ? Is it really required with capybara integration ?

I really like the capybara syntax, it makes much clearer request specs and it's a full win with a JS driver. Did you take a look at poltergeist (there's a good railscast about it too) ? I had a little more luck with it in some cases..

So +1 for this change or any other change that makes it more pleasant to write tests (rspec anyone?).

Thanks for the feedback Jean-Baptiste!

Jean-Baptiste Barth wrote:

Same for database_cleaner no ? Is it really required with capybara integration ?

It is because the browser side doesn't support transaction; the database cleaner is documented by Capybara and delete data right after the test ended (or before it starts but I don't think so).

Did you take a look at poltergeist (there's a good railscast about it too) ? I had a little more luck with it in some cases..

It was my preferred way when I first met PhantomJS and before I understood the way GhostDriver works.

Poltergeist has some disadvantages: first it does not support running on Windows or Mac OS even if I could make it work with Windows with 2 simple modifications (I ran the test suite to make sure it was ok then).
Second it does not support Capybara 2.x (not a big deal though) and mainly it is maintained only by its author with the help of PR.

On the other hand the Selenium driver doesn't have any of these disadvantages, Selenium stands as a reference in browser automation, is used and actively maintained by a big community for almost every language and the Capybara driver is the default one.
Moreover it's very few configuration change to have tests running with FF, IE or Chrome which can be useful to actually see the test running.

After applying the patch and bundle install, ruby test\integration\issues_test.rb throws some Errno::ECONNREFUSED errors. What else do I need to setup?

Jean-Philippe Lang wrote:

After applying the patch and bundle install, ruby test\integration\issues_test.rb throws some Errno::ECONNREFUSED errors. What else do I need to setup?

You need to have PhantomJS 1.8+ running and listening on port 4444. It can be done by simply running phantomjs --webdriver 4444 IIRC.

Jean-Baptiste Barth wrote:

So +1 for this change or any other change that makes it more pleasant to write tests (rspec anyone?).

• from a usability POV, will my basic IDE be able to display the file outline if the test is not a classic Ruby function definition such as test 'add_new_issue' do … end (it don't with shoulda)?
• will I be able to simply run a specific test as with ruby test.rb -n test_add_new issue?
• is really the syntax worth it? I mean:

feature "issue preview" do
  post "/preview", :description => 'blablabla'
  it "should display preview" 
    assert_select "div#preview fieldset", :text => 'blablabla'
  end
end

is not a lot more understandable than:

test preview_issue
  post "/preview", :description => 'blablabla'
  assert_select "div#preview fieldset", :text => 'blablabla'
end

But I would be happy to use it if you could address my concerns.

Etienne Massip wrote:

Did you take a look at poltergeist (there's a good railscast about it too) ? I had a little more luck with it in some cases..

It was my preferred way when I first met PhantomJS and before I understood the way GhostDriver works.

Poltergeist has some disadvantages: first it does not support running on Windows or Mac OS even if I could make it work with Windows with 2 simple modifications (I ran the test suite to make sure it was ok then).

Not sure about windows, but actually the railscasts shows poltergeist working on Macosx, and it works on Linux (at least Ubuntu).

Second it does not support Capybara 2.x (not a big deal though)

Any pointer is welcome about that, I was about to use it in one project, too bad if it doesn't work :/

and mainly it is maintained only by its author with the help of PR.

That's a valid concern ;)

Oh and about RSpec, I was just kidding, your opinion is far more important than mine since I don't contribute code on a regular basis while you do.

Just one tip, it brings no benefit if you use it like test/unit. But if you consider, stub, mock expectations, and all the things that could replace the (unmaintained..) shoulda macros, then it can be interesting. Give it a chance ;) We can tchat about that an other day or by email if you want..

Jean-Baptiste Barth wrote:

Not sure about windows, but actually the railscasts shows poltergeist working on Macosx, and it works on Linux (at least Ubuntu).

I said that by memory but was wrong about it.

Second it does not support Capybara 2.x (not a big deal though)

Any pointer is welcome about that, I was about to use it in one project, too bad if it doesn't work :/

Easy one: https://github.com/jonleighton/poltergeist/issues/136.

Jean-Baptiste Barth wrote:

Oh and about RSpec, I was just kidding, your opinion is far more important than mine since I don't contribute code on a regular basis while you do.

That is a bit exaggerated, you might be a more regular Ruby colder than I am.

We can tchat about that an other day or by email if you want..

Right, IRL then =)

Etienne Massip wrote:

You need to have PhantomJS 1.8+ running and listening on port 4444. It can be done by simply running phantomjs --webdriver 4444 IIRC.

Thanks, I installed PhantomJS 1.8 but still have no luck when running test\integration\issues_test.rb against current trunk:

# Running tests:

..E......F.

Finished tests in 44.281081s, 0.2484 tests/s, 0.8582 assertions/s.

  1) Error:
test_issue_attachments(IssuesTest):
RuntimeError: files/testfile.txt file does not exist
    c:/utils/ruby/lib/ruby/gems/1.9.1/gems/rack-test-0.6.2/lib/rack/test/uploade
d_file.rb:21:in `initialize'
    c:/utils/ruby/lib/ruby/gems/1.9.1/gems/actionpack-3.2.11/lib/action_dispatch
/testing/test_process.rb:38:in `new'
    c:/utils/ruby/lib/ruby/gems/1.9.1/gems/actionpack-3.2.11/lib/action_dispatch
/testing/test_process.rb:38:in `fixture_file_upload'
    c:/utils/ruby/lib/ruby/gems/1.9.1/gems/actionpack-3.2.11/lib/action_dispatch
/testing/integration.rb:382:in `method_missing'
    C:/workspace/trunk/test/test_helper.rb:111:in `uploaded_test_file'
    test/integration/issues_test.rb:105:in `test_issue_attachments'
    c:/utils/ruby/lib/ruby/gems/1.9.1/gems/mocha-0.12.3/lib/mocha/integration/mi
ni_test/version_230_to_2101.rb:28:in `run'

  2) Failure:
test_update_issue_form(IssuesTest) [C:/workspace/trunk/test/test_helper.rb:56]:
<"/login"> expected but was
<"/issues/new">.

11 tests, 38 assertions, 1 failures, 1 errors, 0 skips

Jean-Philippe Lang wrote:

Thanks, I installed PhantomJS 1.8 but still have no luck when running test\integration\issues_test.rb against current trunk:

The patch overrides default #log_user and contains 2 examples, test_add_issue is the conversion of an existing test and test_preview_issue a new test demoing the use of the JavaScript capabilities.

I did not convert every test in case the whole idea was simply rejected and the ones using #log_user probably fail now because of that.

Explicitly calling ActiveSupport#log_user should fix them, I'll have a quick look.

Etienne Massip wrote:

Explicitly calling ActiveSupport#log_user should fix them

Except Ruby doesn't allow that. Renaming ActionDispatch::IntegrationTest#log_user to #log_user_capybara indeed fixes the failure but the first error is still here.

Must be due to the removal of ActionDispatch::TestProcess from ActiveSupport::TestCase.

It works OK now, thanks. But disabling transactional fixtures for all integration tests (including API tests) is not acceptable. We should subclass ActionDispatch::IntegrationTest and maybe isolate these tests in a new test set (eg. ui).

Jean-Philippe Lang wrote:

But disabling transactional fixtures for all integration tests (including API tests) is not acceptable.

Disabling transactions but withe the use of database cleaner is pretty much the same of using transactions, am I wrong? Instead of a ROLLBACK, you've got a DELETE.

We should subclass ActionDispatch::IntegrationTest and maybe isolate these tests in a new test set (eg. ui).

None of the integration tests use ActionDispatch::IntegrationTest for now but ActionController::IntegrationTest.
Not sure it is correct since maybe they should use either ActionDispatch::IntegrationTest or ActionController::TestCase, ActionController::IntegrationTest being a deprecated class.
API tests are the exception and I don't know if they're true integration tests or functional tests.

Etienne Massip wrote:

Disabling transactions but withe the use of database cleaner is pretty much the same of using transactions, am I wrong? Instead of a ROLLBACK, you've got a DELETE.

Sure, it works but running the integration tests with the patch applied takes maybe 20 or 30 minutes when it takes 50s with transactional fixtures.

None of the integration tests use ActionDispatch::IntegrationTest for now but ActionController::IntegrationTest.

Have a look at Rails source: action_controller/deprecated/integration_test.rb :-)

ActionController::IntegrationTest = ActionDispatch::IntegrationTest

API tests are the exception and I don't know if they're true integration tests or functional tests.

Some integration tests should be moved to functional tests indeed, but API tests hit plain paths and routing is involved. They have to stay in integration tests.

Jean-Philippe Lang wrote:

Have a look at Rails source: action_controller/deprecated/integration_test.rb :-)

[...]

God damn it. Ok then, let's subclass ActionDispatch::IntegrationTest.
If you're ok then you just have to start phantomjs on CI and we can start committing this?

I've committed the first 2 tests and installed PhantomJS on the CI server.
Capybara tests are not run by default, you can run them with `rake test:ui`

Is it supposed to be 1.8 compatible? Because tests are failing with ruby1.8 and jruby1.6.7.

The 2 find assertions raise an error although the resulting page contains the element that is matched.
Looks like it's not compatible with 1.8.

Tests disabled for now on the CI server until this problem is solved.

Jean-Philippe Lang wrote:

The 2 find assertions raise an error although the resulting page contains the element that is matched.
Looks like it's not compatible with 1.8.

Indeed, Capybara 2.0 has drop support for 1.8.7 :[

If we keep integration tests it might be OK to run ui tests only on 1.9 since we're testing the browser side which does not involve Ruby?

Etienne Massip wrote:

If we keep integration tests it might be OK to run ui tests only on 1.9 since we're testing the browser side which does not involve Ruby?

I had this in mind anyway because these tests without transactional fixtures are really slow. It's not worth running them with all ruby/database configurations.

The capybara tests now run on the CI server, but there's an issue with #test_preview_issue_description. All #visit calls (including those in the tests that follow) do nothing after fill_in 'Description'... call (test.log confirms that nothing happens). I have no idea about that and had to skip this test.

• http://www.redmine.org/builds/logs/build_trunk_sqlite3_ruby-1.9.3-p194_1170.html
• http://www.redmine.org/builds/logs/build_trunk_sqlite3_ruby-1.9.3-p194_1169.html

For now, I've disabled Capybara tests in r11345.

Jean-Philippe Lang wrote:

The capybara tests now run on the CI server, but there's an issue with #test_preview_issue_description. All #visit calls (including those in the tests that follow) do nothing after fill_in 'Description'... call (test.log confirms that nothing happens). I have no idea about that and had to skip this test.

I think that could be caused by the leave page? confirm box.

• http://www.redmine.org/builds/logs/build_trunk_mysql_ruby-1.8.7-p370_1176.html
• http://www.redmine.org/builds/logs/build_trunk_sqlite3_ruby-1.8.7-p370_1176.html
• http://www.redmine.org/builds/logs/build_trunk_sqlite3_ruby-1.9.2-p318_1176.html (passed)

Jean-Philippe Lang wrote:

The form is now submitted after the preview but random failures still happen, eg:

• http://www.redmine.org/builds/logs/build_trunk_mysql_ruby-1.8.7-p370_1176.html
• http://www.redmine.org/builds/logs/build_trunk_sqlite3_ruby-1.8.7-p370_1176.html
• http://www.redmine.org/builds/logs/build_trunk_sqlite3_ruby-1.9.2-p318_1176.html (passed)

I thought you wouldn't try to make it run with 1.8?

Indeed, I changed the jenkins configuration without taking care of it. But still it failed with jruby1.7.2:
http://www.redmine.org/builds/logs/build_trunk_sqlite3_jruby-1.7.2_1176.html

Random failures on MRI 1.9.3 or JRuby 1.7.2 should not happen; do you people see 1.9.3 them somewhere else than on CI?

Maybe due to a p194 bug?

Maria Tikhonova wrote:

What about to add a task for plugins ui tests?

Committed in r14891 (#21361), thanks.