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

Compute File.writeable based on FileWrite instead of FileRead. #2698

Merged
merged 3 commits into from
May 16, 2018

Conversation

sgebbie
Copy link
Contributor

@sgebbie sgebbie commented May 11, 2018

File._descriptor(...) was computing the writeable flag by checking
if the FileRead capability was set, rather than the FileWrite
capability.

`File._descriptor(...)` was computing the `writeable` flag by checking
if the `FileRead` capability was set, rather than the `FileWrite`
capability.
@jemc jemc added the changelog - fixed Automatically add "Fixed" CHANGELOG entry on merge label May 11, 2018
@jemc jemc changed the title Compute File.writeable based on FileWrite Compute File.writeable based on FileWrite instead of FileRead. May 11, 2018
@sgebbie
Copy link
Contributor Author

sgebbie commented May 14, 2018

Hi,

Sorry, the test for this pull request accidentally depend on: #2697.

I could either pull in the relevant change from #2697, or just wait for #2697 to be merged?

Thanks,
Stewart.

`try ... then ... end` can mask errors and give a false positive to
tests.
@mfelsche
Copy link
Contributor

Hi @sgebbie , this one if failing on OSX with the following logs:

**** FAILED: files/File.open-dir-not-writeable
/Users/travis/build/ponylang/ponyc/packages/files/_test.pony:428: Assert true failed. 
/Users/travis/build/ponylang/ponyc/packages/files/_test.pony:429: Assert false failed. 

@mfelsche
Copy link
Contributor

restarted the job that fails, lets see if this works now.

@sgebbie
Copy link
Contributor Author

sgebbie commented May 16, 2018

@mfelsche Thanks for restarting the job. Now that the directory change is merged, it looks like this is behaving correctly.

@mfelsche mfelsche merged commit 024ce8a into ponylang:master May 16, 2018
ponylang-main added a commit that referenced this pull request May 16, 2018
@mfelsche
Copy link
Contributor

Thanks for your work on this! This is highly appreciated :)

dipinhora pushed a commit to dipinhora/ponyc that referenced this pull request Jun 5, 2018
…ang#2698)

* Compute File.writeable based on FileWrite

`File._descriptor(...)` was computing the `writeable` flag by checking
if the `FileRead` capability was set, rather than the `FileWrite`
capability.

* Catch "inner" errors in the writeable test.

`try ... then ... end` can mask errors and give a false positive to
tests.

* Tidy code style to comply with stdlib guidelines.

Space after : before type.
dipinhora pushed a commit to dipinhora/ponyc that referenced this pull request Jun 5, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
changelog - fixed Automatically add "Fixed" CHANGELOG entry on merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants