Issue 6060 in pharo: New some ensure methods to FileSystem

Status: Accepted
Owner: marianopeck
Labels: Type-Bug

New issue 6060 by marianopeck: New some ensure methods to FileSystem
http://code.google.com/p/pharo/issues/detail?id=6060

- Added #deleteIfAbsent: , #ensureDeleted and #ensureClosed.
- Added #testDeleteIfAbsent, #testEnsureClosed, #testEnsureDeleted and  
#testExists.
- Make FileSystemHandleTest  >> tearDown more robust by using ensure*  
methods.

I have know much about FileSystem so if someone can check please. I don't  
know if the classes where I place the unit tests are the correct place.




_______________________________________________
Pharo-bugtracker mailing list
[hidden email]
http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-bugtracker

Updates:
        Status: FixReviewNeeded
        Cc: [hidden email]
        Labels: -Type-Bug Type-Enh Milestone-2.0

Comment #1 on issue 6060 by marianopeck: New some ensure methods to  
FileSystem
http://code.google.com/p/pharo/issues/detail?id=6060

Slice in inbox.

Name:  
SLICE-Issue-6060-New-some-ensure-methods-to-FileSystem-MarianoMartinezPeck.1
Author: MarianoMartinezPeck
Time: 13 June 2012, 10:08:51.899 am
UUID: e82b6ae6-2342-43ec-85d9-642b89feb015
Ancestors:
Dependencies: FileSystem-Tests-Core-MarianoMartinezPeck.12,  
FileSystem-Core-MarianoMartinezPeck.15

- Added #deleteIfAbsent: , #ensureDeleted and #ensureClosed.
- Added #testDeleteIfAbsent, #testEnsureClosed, #testEnsureDeleted and  
#testExists.
- Make FileSystemHandleTest  >> tearDown more robust by using ensure*  
methods.


_______________________________________________
Pharo-bugtracker mailing list
[hidden email]
http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-bugtracker

Comment #2 on issue 6060 by [hidden email]: New some ensure methods  
to FileSystem
http://code.google.com/p/pharo/issues/detail?id=6060

looks good.


BTW: no need to create an additional block for ifTrue:, the following is  
perfectly valid ;):

deleteIfAbsent: aBlock
        self exists
                ifTrue: [self delete]
                ifFalse: aBlock



_______________________________________________
Pharo-bugtracker mailing list
[hidden email]
http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-bugtracker

Comment #3 on issue 6060 by [hidden email]: New some ensure methods  
to FileSystem
http://code.google.com/p/pharo/issues/detail?id=6060

See what you think... I made Cami's change and beefed up the tests a  
bit, "ensuring" :) that both branches of all ensureXxx were tested...

SLICE-Issue-6060-New-some-ensure-methods-to-FileSystem-SeanDeNigris.2
* enhanced tests
* refactored #deleteIfAbsent: per Cami's comment in issue
...

The only test I'm not sold on is:
testEnsureClosed
        filesystem := self createFileSystem.
        reference := filesystem * 'plonk'.
        handle := reference openWritable: true.
        reference delete.
        handle reference exists ifTrue: [self error].
        handle ensureClosed.
" self deny: handle isOpen."

If the last line is uncommented, the test fails. Also, what exactly are we  
testing? That an error is not signaled? It's not explicit.


_______________________________________________
Pharo-bugtracker mailing list
[hidden email]
http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-bugtracker

Comment #4 on issue 6060 by marianopeck: New some ensure methods to  
FileSystem
http://code.google.com/p/pharo/issues/detail?id=6060

Hi Sean and Cami. Thanks for checking :)

"Also, what exactly are we testing? That an error is not signaled? It's not  
explicit."

yes, we are testing that. I didn't make it explicit because FS does NOT  
throw an specific error but instead a FileDoesNotExist. We could do

self shouldnt: [handle ensureClosed ] raise: FileDoesNotExist.

and hope that it does not throw that error for another reason.


I have no idea why "handle isOpen." answers true even if we close it.  
Camillo?




_______________________________________________
Pharo-bugtracker mailing list
[hidden email]
http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-bugtracker

Updates:
        Cc: [hidden email]

Comment #5 on issue 6060 by marianopeck: New some ensure methods to  
FileSystem
http://code.google.com/p/pharo/issues/detail?id=6060

(No comment was entered for this change.)


_______________________________________________
Pharo-bugtracker mailing list
[hidden email]
http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-bugtracker

Comment #6 on issue 6060 by [hidden email]: New some ensure methods  
to FileSystem
http://code.google.com/p/pharo/issues/detail?id=6060

I would avoid all handle tests so far. since we don't use them as they are  
supposed to.
In this case I would simply skip the test with a comment on future  
implementation...


_______________________________________________
Pharo-bugtracker mailing list
[hidden email]
http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-bugtracker

Comment #7 on issue 6060 by marianopeck: New some ensure methods to  
FileSystem
http://code.google.com/p/pharo/issues/detail?id=6060

so Sean what do we do? do you wanna add the shouldnt:raise: or we integrate  
it as it is?


_______________________________________________
Pharo-bugtracker mailing list
[hidden email]
http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-bugtracker

Comment #8 on issue 6060 by [hidden email]: New some ensure methods  
to FileSystem
http://code.google.com/p/pharo/issues/detail?id=6060

let me know when this is ready.


_______________________________________________
Pharo-bugtracker mailing list
[hidden email]
http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-bugtracker

Comment #9 on issue 6060 by [hidden email]: New some ensure methods  
to FileSystem
http://code.google.com/p/pharo/issues/detail?id=6060

I made one more version... I didn't comment out the handle test as cami  
suggested, because there is a whole FileSystemHandleTest class. We would  
have to stop the whole class from running (#isAbstract?), which should be a  
separate issue.

SLICE-Issue-6060-New-some-ensure-methods-to-FileSystem-SeanDeNigris.3

Version 3:
* moved FileReference tests from FileSystemHandleTest to FileReferenceTest
* enhanced FileSystemHandleTest>>testEnsureClosed to explicitly assert no  
error was signaled

Version 2:
* enhanced tests
* refactored #deleteIfAbsent: per Cami's comment in issue

Version 1:
- Added #deleteIfAbsent: , #ensureDeleted and #ensureClosed.
- Added #testDeleteIfAbsent, #testEnsureClosed, #testEnsureDeleted and  
#testExists.
- Make FileSystemHandleTest  >> tearDown more robust by using ensure*  
methods.


_______________________________________________
Pharo-bugtracker mailing list
[hidden email]
http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-bugtracker

Comment #10 on issue 6060 by [hidden email]: New some ensure methods  
to FileSystem
http://code.google.com/p/pharo/issues/detail?id=6060

in 20131


_______________________________________________
Pharo-bugtracker mailing list
[hidden email]
http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-bugtracker

Updates:
        Status: Integrated

Comment #11 on issue 6060 by [hidden email]: New some ensure methods  
to FileSystem
http://code.google.com/p/pharo/issues/detail?id=6060

(No comment was entered for this change.)


_______________________________________________
Pharo-bugtracker mailing list
[hidden email]
http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-bugtracker

Comment #12 on issue 6060 by marianopeck: New some ensure methods to  
FileSystem
http://code.google.com/p/pharo/issues/detail?id=6060

Thanks Sean!!!! :)  I love this good energy.


_______________________________________________
Pharo-bugtracker mailing list
[hidden email]
http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-bugtracker

Comment #13 on issue 6060 by [hidden email]: New some ensure methods  
to FileSystem
http://code.google.com/p/pharo/issues/detail?id=6060

This is fun, isn't it?!


_______________________________________________
Pharo-bugtracker mailing list
[hidden email]
http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-bugtracker