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

remove features' directories on windows with proper access #5389

Merged
merged 1 commit into from
Sep 21, 2016

Conversation

ashmaroli
Copy link
Member

@ashmaroli ashmaroli commented Sep 21, 2016

Feature directories generated during cucumber tests currently throw a Errno::EACCES exception after every scenario runs on windows.
This Pull resolves this issue.

/cc @jekyll/windows

@XhmikosR
Copy link
Contributor

Which tests does this fix exactly? The cucumber one?

@ashmaroli
Copy link
Member Author

Which tests does this fix exactly? The cucumber one?

yes.. thought you'd understand if I just mentioned features
I'll update the body..

@XhmikosR
Copy link
Contributor

It works as far as I can tell

before:

201 scenarios (201 failed)
1638 steps (7 failed, 2 skipped, 1629 passed)

after:

201 scenarios (7 failed, 194 passed)
1638 steps (7 failed, 2 skipped, 1629 passed)

@XhmikosR
Copy link
Contributor

But there seems to be a failure in Travis.

@@ -1,12 +1,13 @@
Before do
FileUtils.rm_rf(Paths.test_dir) if Paths.test_dir.exist?
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added this line to ensure every scenario starts with a clean slate

@ashmaroli
Copy link
Member Author

But there seems to be a failure in Travis.

Thats a known bug.. happens randomly to random suites.. Maintainers can restart that test to get it to pass..
Cucumber passed with Green..

Copy link
Member

@parkr parkr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for this! I restarted the failing Travis build. Once that's good, we can merge.

FileUtils.mkdir_p(Paths.test_dir) unless Paths.test_dir.directory?
Dir.chdir(Paths.test_dir)
end

#

After do
Paths.test_dir.rmtree if Paths.test_dir.exist?
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Out of curiosity, why does .rmtree fail? Can you paste the error or describe the issue? It should be identical to FileUtils.rm_rf IIRC.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

.rmtree does not fail per se. The feature directories do get removed completely (verified locally), but on Windows it also raises an alarm:

  Permission denied @ dir_s_rmdir - C:/projects/jekyll/tmp/jekyll (Errno::EACCES)
  C:/projects/jekyll/features/step_definitions.rb:9:in `After'

I stumbled upon the fix when I tried the identical FileUtils.rm_rf method. This method manipulates files rather than try to modify the directory attribute.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Interesting! @envygeeks, is this a part of Pathutil?

@parkr
Copy link
Member

parkr commented Sep 21, 2016

@jekyllbot: merge +dev

@jekyllbot jekyllbot merged commit 25d4291 into jekyll:master Sep 21, 2016
@parkr parkr added the tests label Sep 21, 2016
jekyllbot added a commit that referenced this pull request Sep 21, 2016
@ashmaroli ashmaroli deleted the cucumber-with-access branch September 22, 2016 02:15
@jekyll jekyll locked and limited conversation to collaborators Jul 11, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants