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
Test failures #4
Comments
|
Hi Utkarsh, Thanks for using the gem. Looking at the output, I can see that the failure is not caused by the |
|
Hi @piotrmurach, I'm running into this as well, and don't understand your recommendation: The version of Any further idea? Thanks! |
|
Hi @ge-fa, To run specs you need to preload all dependencies, the easiest and most common way in Ruby world is to use So using bundler and rake run the following: Alternatively, you can skip using rake and run |
|
Thanks, I'm aware of the workings of Note: This issue is not only related to this lib, it poped up elsewhere as well, see for example this issue. |
|
I run the test suite on Ruby allow(::File).to receive(:join).with(...).and_return(...)fails due to arguments to allow(::File).to receive(:join).and_return(...)This makes the test pass. But what is the point of a test that doesn't test the implementation? I feel as though here we're working to fix how rspec works on Ruby When you look closely at the error message: You can see that's something to do with Rubygems threading issues and thus failing to load files correctly on Ruby Bundler is a default gem packaged with Ruby. This issue doesn't affect how the test suite works in my current workflow that uses bundler. The tests are here to support releases. I'm in two minds about changing tests and making them less useful to work around a bug in Ruby 2.7 and Rubygems integration. |
|
Thanks for your follow-up and your findings, I appreciate this a lot. I'll try to find the cause and see what we're able to do in Debian about this. |
|
Thanks for making me dig a little bit deeper into this problem. I'm not necessarily sure what the core problem is here but everything seems to point to some issue with how rubygems loads dependencies in Ruby |
|
ACK, thanks, I'll let you know, in case I'm able to find anything valuable. |
|
(forwarded this issue to the rubygems folk!) 😄 Also, similar issues: |
|
@utkarsh2102 Could you provide the link to the rubygems issue? |
|
@utkarsh2102 The |
|
@ge-fa, |
|
@utkarsh2102 Thanks! |
|
Hello!! 👋 👋 This is quite convoluted 😆. So, this is the backtrace I get printed together with the "CRITICAL: RUBYGEMS_ACTIVATION_MONITOR.owned?: before false -> after true" error: If we observe the backtrace bottom-up, we see the following stuff:
So, maybe there's an issue in But probably the easiest way out is to use The other failures should be similar. Hope it helps! |
|
@deivid-rodriguez Thank you for the quick reply and your investigation! I have reservations about concluding that rubygems is not at fault here. I'm not saying it is but I also cannot see evidence that it isn't. I think this is a bug indicative of some significant regression someplace and it may be that it is manifested in RSpec failure but the root cause is someplace else. Let's look at the provided example snippet and the comment:
#<File (class)> received :file? with unexpected arguments
expected: ("ruby")
got: ("/home/deivid/Code/tty-which/spec/rspec/matchers/built_in/eql")
Please stub a default value first if message might be received with other args as well. The method definition is as simple as delegating passed value: def executable_file?(filename)
::File.file?(filename) && ::File.executable?(filename)
endand the corresponding test looks like this: allow(::File).to receive(:file?).with("ruby").and_return(true)
allow(::File).to receive(:executable?).with("ruby").and_return(true)
expect(TTY::Which.executable_file?("ruby")).to eql(true)Can you please explain to me how Using the allow(File).to receive(:exist?)Which matches the automated suggestion from the failure # allow(::File).to receive(:file?).with("ruby").and_return(true)
# replaced with
allow(::File).to receive(:file?).with(any_args).and_return(true)
allow(::File).to receive(:executable?).with("ruby").and_return(true)
expect(TTY::Which.executable_file?("ruby")).to eql(true)But we haven't solved the problem and made the test 'weaker' as it no longer tests that My other question is that, if this is Hi @JonRowe - do you have any experience with the above? Does anything jump out for you as the cause of the issue? It's been suggested that |
It's rubygems making that call, it's not your code. Rubygems code gets run because |
|
I honestly think there's no good solution here. Even if rubygems wouldn't monkeypatch |
I'm not saying that this is an RSpec issue, or that you should change your code, I'm just trying to explain the situation. The problem is actually not dependent on ruby 2.7, but on the |
If there are different versions of files being loaded its because the host system has more than one version of them installed and bundler is not being used, Bundler solves many of these problems by manipulating the path to be correct for the current installed versions of dependencies. Without it you must do this manually. No other package manager is currently aware of Ruby dependencies to my knowledge. |
|
Just to clarify my suggestion, I meant to add on top of the other mocks, not instead of them, so in case other code external to your library, be it And I also meant it as a patch to be added by
Also, this was just a quick thought, but it sounds like something quite hard to implement, for little benefit.
Actually, rubygems But unfortunately, it uses |
|
@deivid-rodriguez Thanks a lot for explaining this. It makes a lot more sense now. I see also why you're suggesting to use: @JonRowe Would it make sense and is it even possible for |
|
@deivid-rodriguez I read your first comment with all the backtraces in more detail and they make perfect sense now. It seems like mocking It seems like |
Great! This should be a rare case, since it needs specs not be run with
But then users would miss actual unintended calls with other arguments to |
Rubygems also manipulates your path... its also intertwined with bundler these days. There are still issues if it is not limited to the current Gemfile's dependencies. Using gems from linux package managers is known to be generally problematic.
No it is not possible, we don't know wether the code is intentionally called or unintentionally called. Stubs only exist for the lifecycle of your test, they are cleaned up automatically so they are directly or indirectly invoked by you.
If you stub a core method you are responsible for restoring any behaviour you depend upon elsewhere in your tests. This can be as simple as: It is an impossible task to ask RSpec to special case all of the core library to ensure some special behaviour.
This is an interesting idea but in practise I don't think it would work, we only know wether code is in the current working directory, or not, so much core Ruby behaviour is now gem based we wouldn't be able to distinguish between core Ruby libraries and other gems, and most people stub gems not Ruby features. |
Yeah, even if the first I don't think there's anything actionable here other than people adding their workaround of choice like |
I don't think
Indeed, but it's getting better. The Debian ruby team has packaged hundreds of gems which are used by people. And they seem to work 😆. I think it's a great way to make general purpose ruby tools more widely available and I really appreciate the work they do ❤️. |
It's not just your ruby version manager doing it, its the entire system, at the end of the day the load path has usually been touched by several things and has an order. The first file matching your require in the load path will be the one that is loaded.
I was attempting (badly it seems) to explain why |
|
Sorry, I understand that you said "path" where you meant "load path". In that case, sure, I already mentioned this in previous comments. |
I think we all got why this happens at this point 👍 |
|
@deivid-rodriguez I decided to fix the failures to help make Debian releases easier. Thanks again for investigating and explaining the issue in such detail, that was mega useful. @JonRowe Thanks for your perspective, I ensured that the original version of the mocked call is executed. |
|
That's really kind of you @piotrmurach 💜! |
Hi,
There are some test failures:
Any clue about the fix?
The text was updated successfully, but these errors were encountered: