Is it possible to add a test for this?

@dentarg I was thinking about it while implementing it, but the only two options of triggering this would be to either add zeitwerk as a dependency (only in test?) or stub Kernel#caller and neither of those feels particularly appealing to me. I was wondering if that's the reason there never were any tests for this functionality to begin with.

If you're OK with one of these solutions then please let me know which one you'd prefer and I'll add a spec using that approach. Alternatively let me know if you can see another way :)

add zeitwerk as a dependency (only in test?)

I was thinking about this solution – yes, only as a test dependency, there's a bunch of other dependencies for tests so that should be fine IMHO.

@dentarg I tried testing this, but I'm afraid I hit a wall. zeitwerk is inside callers, but only once on require 'sinatra/base' in test/helper.rb That's before any tests run so if I do something like assert_equal __FILE__, TestApp.app_file it won't work since on subsequent require calls zeitewerk doesn't appear as a caller. I think the only way to trigger this would be to have a completely separate test file that does not include the helper and I'm not even sure that's possible? Let me know if I'm missing something :)

Sorry, I don't know, I would have to give it a try it myself :)

Can you help me understand the issue? Or how to reproduce it to better undertand?

Once you load Zeitwerk, all calls to Kernel#require pass through Zeitwerk. The only way this cannot happen is that someone later on resets Kernel#require to some older version of it, thus removing anyone else decoration. This is technically possible, but you'd be removing decorations which are out of your ownership, so it would be a bad practice.

In any case, Sinatra wants to find the application file among the callers. Whether Zeitwerk is there or not should not matter, no? You walk the callers up until you hit one that does not match any of the patterns. In that logic, patterns that are not found do not conflict, they just won't ever match.

Apologies for late reply, finally had some time to look into this.

@fxn you're absolutely right, this had nothing to do with Zeitwerk. It turns out the bug is only reproducible using Sinatra's classic approach and the test I was trying to extend was using modular.

@dentarg I took a stab at this in a separate file since it has to be in classic style in order for this bug to appear, but that messes up with another test and I'm not really sure how to handle that conflict. I committed the test on a separate branch for now, see DawidJanczak@9abff65 The commit message describes the problem, I would appreciate any input you might have on this :)

@dentarg I spent some more time on this and I'm not sure it's possible to make it work. The failing tests (https://github.com/sinatra/sinatra/blob/master/test/settings_test.rb#L238 and https://github.com/sinatra/sinatra/blob/master/test/settings_test.rb#L506) seem to depend on the file that no classic style app is defined and I need to define that in order to test this bug. The README says "The main disadvantage of using the classic style rather than the modular style is that you will only have one Sinatra application per Ruby process." so I guess it makes sense that this doesn't work. Perhaps that's why there were no tests for this functionality in the first place.

Let me know if I'm misunderstanding this please.

@DawidJanczak thank you for your perseverance. I'm just going to merge without a test, I appreciate the effort you put into trying to find a way to add one.

@DawidJanczak Sorry for never getting back on this, I added test for Zeitwerk in #1975