Skip to content

Add ERROR_FILE_EXISTS to ErrorKind conversion on Windows#34270

Merged
bors merged 3 commits into
rust-lang:masterfrom
gkoz:error_file_exists
Jun 16, 2016
Merged

Add ERROR_FILE_EXISTS to ErrorKind conversion on Windows#34270
bors merged 3 commits into
rust-lang:masterfrom
gkoz:error_file_exists

Conversation

@gkoz

@gkoz gkoz commented Jun 14, 2016

Copy link
Copy Markdown
Contributor

@Stebalien

Copy link
Copy Markdown
Contributor

How hard would it be to add a platform-independent test case for this (so it doesn't show up again)?

@gkoz

gkoz commented Jun 14, 2016

Copy link
Copy Markdown
Contributor Author

Sorry, it's not obvious to me whether by 'this' you mean inaccurate ErrorKind::Other in general or something more specific.

@alexcrichton

Copy link
Copy Markdown
Member

I believe what @Stebalien means is could we add a test for this? Ideally one that runs on both Unix and Windows and both situations give us AlreadyExists

@Stebalien

Copy link
Copy Markdown
Contributor

I believe what @Stebalien means is could we add a test for this? Ideally one that runs on both Unix and Windows and both situations give us AlreadyExists

Ideally one that doesn't check for any specific platforms.

@gkoz

gkoz commented Jun 14, 2016

Copy link
Copy Markdown
Contributor Author

Added a test.

@alexcrichton

Copy link
Copy Markdown
Member

@bors: r+ 973ec7cfa9a14bc825102188e8ad40ff3d733cda

Thanks!

@bors

bors commented Jun 15, 2016

Copy link
Copy Markdown
Collaborator

⌛ Testing commit 973ec7c with merge ccd3352...

@bors

bors commented Jun 15, 2016

Copy link
Copy Markdown
Collaborator

💔 Test failed - auto-win-gnu-32-opt-rustbuild

Comment thread src/libstd/fs.rs Outdated

@gkoz gkoz Jun 15, 2016

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Apparently on Windows an this (creating a file when a directory exists) results in ErrorKind::PermissionDenied. Should I just not test this or make it conditional on !windows?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

It's fine to just omit this test if the PR is covered by the one above.

@gkoz

gkoz commented Jun 15, 2016

Copy link
Copy Markdown
Contributor Author

Removed fancy test cases.

@alexcrichton

Copy link
Copy Markdown
Member

@bors: r+

@bors

bors commented Jun 15, 2016

Copy link
Copy Markdown
Collaborator

📌 Commit 552afd3 has been approved by alexcrichton

bors added a commit that referenced this pull request Jun 16, 2016
Rollup of 4 pull requests

- Successful merges: #34207, #34268, #34270, #34290
- Failed merges:
@bors bors merged commit 552afd3 into rust-lang:master Jun 16, 2016
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.

4 participants