thoughtbot / capybara-webkit

A Capybara driver for headless WebKit to test JavaScript web apps
https://thoughtbot.com/open-source
MIT License
1.97k stars 428 forks source link

Fix the 'Reset' command in debug builds (on Windows) #986

Closed joankaradimov closed 7 years ago

joankaradimov commented 7 years ago

I am trying to get Capybara Webkit to run on my environment (Windows 10, 64 bit with JRuby 9.1.7.0). I use MSYS2 which [among other things] provides a MinGW environment with QT5 binaries and build tools. Everything is 64 bit.

Getting the code to build is fairly straightforward once the environment is all set (with the exception of this issue in Capybara Webkit). But running bundle exec rake results in multiple test failures.

I was able to isolate a simple scenario that fails. The steps are:

  1. Manually run the executable.
C:\dev\capybara-webkit>bin\webkit_server.exe
Capybara-webkit server started, listening on port: 54764
  1. Then in a new terminal launch a jirb and do this:
irb(main):012:0> require 'socket'
irb(main):013:0> x = TCPSocket.open('127.0.0.1', 54764)
=> #<TCPSocket:fd 1108>
irb(main):014:0> x.puts('Reset'); x.puts(0)
  1. Back in the first terminal this error is logged:
ASSERTION FAILED: m_cacheDirectory.isNull()
C:/repo/mingw-w64-qtwebkit/src/qtwebkit-tp5/Source/WebCore/loader/appcache/ApplicationCacheStorage.cpp(369) : void WebCore::ApplicationCacheStorage::setCacheDirectory(const WTF::String&)
1   000000000cdf0ab7
2   000000000cfe78a4
3   000000000ca44a5e
4   0000000000405061
5   000000000041210d
6   0000000000412bad
7   000000000040a200
8   0000000000411412
9   00000000004138e2
10  0000000000413815
11  00000000004094ac
12  0000000000409348
13  000000000041b5e2
14  00000000059e0a6e
15  00000000059e01f6
16  000000000041e624
17  000000000040f6b9
18  000000000040f543
19  000000000040f452
20  000000000040f398
21  000000000041e420
22  00000000059e0a6e
23  00000000059e01f6
24  0000000005a3fca1
25  0000000066cd99e3
26  0000000066cd80ef
27  0000000066d3f3a0
28  0000000066ccba46
29  0000000066ce4fb0
30  0000000019b71315
31  0000000019b6e6d9

I found this issue in the webkit bugtracker in which it is stated that:

[...] the application cache directory can only be set once. If you try to set it more than once on a debug build, you will hit this ASSERT.

... which is exactly what is happening for me. The build that I produced is a debug one. So I moved the cache initialization away from the WebPage constructor.

I suspect that Linux/macOS builds are release ones and are not affected by this. Still if this is an issue as the guys over bugs.webkit.org claim, this best be fixed everywhere.

mhoran commented 7 years ago

Nice find! I wonder if we should just do this in the WebPageManager constructor, instead of calling back from WebPage? We do other initialization and resetting from the WebPageManager, so that seems appropriate -- so long as it can be set up before the first WebPage is created.

joankaradimov commented 7 years ago

I wonder if we should just do this in the WebPageManager constructor, instead of calling back from WebPage?

That was my initial thought too. Unfortunately I could not getting it working.

This test: Capybara::Webkit::Driver localStorage works clears the message after a driver reset! ... is failing unless the cache initialization happens between the connects and the createWindow call in the WebPage constructor. The first call to the constructor to be precise. I could not determine the exact cause.

joankaradimov commented 7 years ago

Ok, I moved some code around. The cache initialization still happens outside the WebPageManager constructor. Bit it is slightly better now, I think.

mhoran commented 7 years ago

Thanks!