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

Projects
None yet
4 participants
@ashmaroli
Member

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

This comment has been minimized.

Show comment
Hide comment
@XhmikosR

XhmikosR Sep 21, 2016

Contributor

Which tests does this fix exactly? The cucumber one?

Contributor

XhmikosR commented Sep 21, 2016

Which tests does this fix exactly? The cucumber one?

@ashmaroli

This comment has been minimized.

Show comment
Hide comment
@ashmaroli

ashmaroli Sep 21, 2016

Member

Which tests does this fix exactly? The cucumber one?

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

Member

ashmaroli commented Sep 21, 2016

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

This comment has been minimized.

Show comment
Hide comment
@XhmikosR

XhmikosR Sep 21, 2016

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)
Contributor

XhmikosR commented Sep 21, 2016

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

This comment has been minimized.

Show comment
Hide comment
@XhmikosR

XhmikosR Sep 21, 2016

Contributor

But there seems to be a failure in Travis.

Contributor

XhmikosR commented Sep 21, 2016

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?

This comment has been minimized.

@ashmaroli

ashmaroli Sep 21, 2016

Member

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

@ashmaroli

ashmaroli Sep 21, 2016

Member

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

@ashmaroli

This comment has been minimized.

Show comment
Hide comment
@ashmaroli

ashmaroli Sep 21, 2016

Member

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..

Member

ashmaroli commented Sep 21, 2016

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..

@parkr

parkr approved these changes Sep 21, 2016

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?

This comment has been minimized.

@parkr

parkr Sep 21, 2016

Member

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.

@parkr

parkr Sep 21, 2016

Member

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.

This comment has been minimized.

@ashmaroli

ashmaroli Sep 22, 2016

Member

.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.

@ashmaroli

ashmaroli Sep 22, 2016

Member

.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.

This comment has been minimized.

@parkr

parkr Sep 22, 2016

Member

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

@parkr

parkr Sep 22, 2016

Member

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

@parkr

This comment has been minimized.

Show comment
Hide comment
@parkr

parkr Sep 21, 2016

Member

@jekyllbot: merge +dev

Member

parkr commented Sep 21, 2016

@jekyllbot: merge +dev

@jekyllbot jekyllbot merged commit 25d4291 into jekyll:master Sep 21, 2016

1 of 2 checks passed

continuous-integration/appveyor/pr AppVeyor build failed
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@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 ashmaroli:cucumber-with-access branch Sep 22, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment