Skip to content
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

Switch checking to see if paths are directories or symlinks #20

Merged
merged 1 commit into from May 8, 2020

Conversation

rjfarmer
Copy link
Collaborator

@rjfarmer rjfarmer commented May 7, 2020

I'd like to be able move the test cases out of the test suite to another hard drive and put a symlink back in place (so mesa test doesn't need to understand new location) But it seem's ruby's File.directory returns false on a symlink. So this switches some of the directory checks for a directory or symlink check. Note this is my first attempt at ruby, and i don't even know how to test mesa_test itself without screwing up my installed version.

@rjfarmer rjfarmer requested a review from wmwolf May 7, 2020 17:42
@jschwab
Copy link
Collaborator

jschwab commented May 7, 2020

Weird. File.directory? is documented to return true on a symlink that points to a directory. https://ruby-doc.org/core-2.5.0/File.html#method-c-directory-3F

@jschwab
Copy link
Collaborator

jschwab commented May 7, 2020

Also, to test, I think you can do:

  • clone the repo
  • edit bin/mesa_test to have DEVELOPMENT_MODE = true (near the top)
  • invoke modified mesa_test directly (i.e. by /path/to/git/bin/mesa_test)

@rjfarmer
Copy link
Collaborator Author

rjfarmer commented May 7, 2020

Serves me right for looking at an old stack overflow post for whats happening (or my complete lack of ruby knowledge making me misunderstand things)

@rjfarmer
Copy link
Collaborator Author

rjfarmer commented May 7, 2020

Closing for now while i dig into the actual issue (and not the one i assumed i had)

@rjfarmer rjfarmer closed this May 7, 2020
@rjfarmer rjfarmer reopened this May 8, 2020
@rjfarmer
Copy link
Collaborator Author

rjfarmer commented May 8, 2020

Okay i understand whats going on and why what i did by mistake in the pull request works (and have tested it as well now thanks, @jschwab )

background: I need to move the test cases out of MESA_DIR onto local hard drives. I then symlink the folder back into MESA_DIR test suite so nothing needs to worry about the different locations. This means that on some nodes when mesa_test runs a test some other tests will have broken symlinks (because there not valid of that node).

When you run mesa_test it calls load_test_source_data which states "allow for brainless loading of all module data" which is perfectly reasonable thing, normally. This iterates over all test cases and sees if the folder is valid (even for test cases that you are not interested in testing because I'm using test_one). But remember the broken symlinks? load_test_source_data fails because a test case your not testing does not exist on the node your using. In a perfect world it would not care about tests its not testing.

File.directory? only succeeds if the symlink is valid so my pull request patches things so its true if the folder exists or the symlink exists (even if not valid). This skips over the issue in load_test_source_data and lets me run the test i want and not fail on tests i don't care about.

@jschwab
Copy link
Collaborator

jschwab commented May 8, 2020

Ah, makes sense. Nice analysis. Seems like your workaround is a reasonable short-term thing and then longer-term (git era) we should make sure test_one only cares about its test.

@wmwolf
Copy link
Collaborator

wmwolf commented May 8, 2020

Looks reasonable. I will do another blind merge and push a new version out to rubygems.

@wmwolf wmwolf merged commit d9c44ba into master May 8, 2020
@wmwolf
Copy link
Collaborator

wmwolf commented May 8, 2020

0.2.14 is live on rubygems

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants