search

btm / minitest-handler-cookbook

Apache License 2.0
59 stars 36 forks source link

Problem with over-keen test file detection in libraries/test_loader.rb #67

Closed ghostflame closed 10 years ago

ghostflame commented 10 years ago

Line 188 Recommend

Consider two recipes: cookbook::users and cookbook::extra_users The test files for extra_users match the end_with? test when you are searching for test files for users. This means running tests for recipes that have not been run; errors abound.

The filenames should all be either 'tests/filename.rb' or 'test/minitest/filename.rb' those being the search locations, so the prepended / should never exclude legitimate matches.

dpetzel commented 10 years ago

Hmm interesting.... I'll try and get a test case put together to flush this out. My initial thoughts/concerns are:

ghostflame commented 10 years ago

Hi,

I don't think it was matching cross cookbook. The issue was one of tail-matching where one recipe name was a substring of another. To require recipe names to bear that restriction seems undesirable. Perhaps use ruby's File methods to pull apart the test file path for a more direct match against "#{recipe_name}_test.rb". That should be Windows-safe...

Sent from my HTC One X on O2

----- Reply message ----- From: "dpetzel" notifications@github.com To: "btm/minitest-handler-cookbook" minitest-handler-cookbook@noreply.github.com Cc: "ghostflame" cybernetic.dragon@gmail.com Subject: [minitest-handler-cookbook] Problem with over-keen test file detection in libraries/test_loader.rb (#67) Date: Wed, Aug 6, 2014 14:57

Hmm interesting.... I'll try and get a test case put together to flush this out. My initial thoughts/concerns are:

/ might break Windows nodes. I don't recall if we do any sanitation or if its possible to have // in the path. The block of code should only be considering test files within the scope of a single cookbook. It should not even be aware of test from a different cookbook. So while your recommendation may just work, I think there might be a larger issues if the test loader is finding tests from a different cookbook than the one it is evaluating. IE it loops through each cookbook one at a time and finds the test files for that cookbook, when working on any single cookbook the list of available test files should only include ones from that cookbook.

— Reply to this email directly or view it on GitHub.

dpetzel commented 10 years ago

I'm not entirely sure I'm understanding the problem context then. The match is confined to a cookbook, so if another cookbook has a similarly named recipe, it shouldn't matter. I'm guessing it somehow is mattering based on your suggestions though. Would it be possible to pass along the error message?

Rucknar commented 10 years ago

I've seen this issue.

Say you have a cookbook used for users 'users_cookbook'.In this cookbook you have a two recipe's. 'third-party-users.rb' & 'users.rb'. Then you assign tests with the names 'third-party-users_test.rb' and 'users_test.rb'.

Your building a host and you included say just the 'users_cookbook::users recipe on a hosts run-list then it matches both because the filename of both tests end with 'users_test.rb'. The match is ignoring the recipe name as a whole and just looking for files ending with the 'recipename_test.rb' Currently it's too ambiguous and should probably be constrained to match the whole filename.

dpetzel commented 10 years ago

ahh.. That makes sense, I totally misread the original report :( I believe I should be able to work up some tests and fix in the not to distant future.

dpetzel commented 10 years ago

@ghostflame @superted666 Can you give the version on master a spin and see if it solves the problem for you?

Thanks