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

Make sure a file/directory does not exist + Cleanup named file/directory steps #234

Merged
merged 2 commits into from May 7, 2015
Merged

Conversation

ghost
Copy link

@ghost ghost commented Apr 8, 2015

I often use an init-command to create some files. In my steps I try to make it obvious that the file should not exist. This PR adds new cucumber steps which makes the intent very clear and does not fail if I file/directory does not exist. This PR also includes a small refactoring - hope this is ok.

New steps

  • Make sure file does not exist
  • Make sure directory does not exist

Cleanup step

  • Make directory step look the same like file step

I'm very well aware of the I remove file/directory "xz.txt"-step. But I think the intention is very different.

@ghost
Copy link
Author

ghost commented Apr 8, 2015

Mmh... Seems like there are some more inconsistencies in step names:

Then /^a file named "([^"]*)" should exist$/ do |file|
  check_file_presence([file], true)
end

Then /^a file named "([^"]*)" should not exist$/ do |file|
  check_file_presence([file], false)
end

Then /^the file "([^"]*)" should not exist$/ do |file_name|
  check_file_presence([file_name], false)
end

@jarl-dk What do you think which wording should be used? I don't think we should support both ones.

@mattwynne
Copy link
Member

I don't think we should support both ones.

Actually, I'd love it if we did. I keep forgetting the 'named' bit when I use these steps in Cucumber's features, and I've always thought it read awkwardly. What about making it consistently optional?

@ghost
Copy link
Author

ghost commented Apr 9, 2015

I keep forgetting the 'named' bit when I use these steps in Cucumber's features,

Me too

What about making it consistently optional?

What do you mean with "consistently optional"?

@mattwynne
Copy link
Member

Something like this:

Then /^a file(?: named)? "([^"]*)" should exist$/ do |file|
  check_file_presence([file], true)
end

@ghost
Copy link
Author

ghost commented Apr 9, 2015

ah. ok. I see. will fix that.

@ghost
Copy link
Author

ghost commented Apr 9, 2015

Ok. I changed some tests to test the new behaviour. Now all steps for directories/files are built like that. I also change the "not"-thing as well to reduce the number of needed steps - a pattern I found in other existing steps.

Then /^(?:a|the) file(?: named)? "([^"]*)" should (not )? exist$/ do |file, expect_match|
  check_file_presence([file], !expect_match)
end

@ghost ghost changed the title Make sure a file/directory does not exist Make sure a file/directory does not exist + Cleanup named file/directory steps Apr 9, 2015
@ghost
Copy link
Author

ghost commented Apr 9, 2015

If you prefer I can split up this PR and make a new one for the newly added steps. Although those steps were the reason I opened this PR. 😄

@ghost
Copy link
Author

ghost commented Apr 16, 2015

@mattwynne @jarl-dk

ok?

@mattwynne
Copy link
Member

LGTM. It might be worth rebasing your commits to keep the history tidy when we merge this in.

@ghost
Copy link
Author

ghost commented Apr 22, 2015

Rebased.

@ghost
Copy link
Author

ghost commented Apr 27, 2015

@mattwynne @jarl-dk Can we go on with this PR?

@ghost
Copy link
Author

ghost commented Apr 30, 2015

Rebased.

mattwynne added a commit that referenced this pull request May 7, 2015
Make sure a file/directory does not exist + Cleanup named file/directory steps
@mattwynne mattwynne merged commit 9c73799 into cucumber:master May 7, 2015
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.

1 participant