Skeleton PathAssert, plus tests -- NOT FOR INCLUSION. Yet.

[fge]

Skeleton based on existing code. I modeled it on FileAssert. In fact I saw nothing related to anything java.nio.file in the code, so I created it from scratch...

Preview only. I know the coding style does not fit; knowing that I use IDEA, do you happen to have a style file available?

Also, about error messages: there seem to be a lot of factories; FileAssert seems to basically use one per message! Is that on purpose? Can I create a single factory for Path-specific operations (including exists, non exists and absolute)?

Please comment! I'll add more test when the base is good enough for you.

[joel-costigliola]

extract the message as a constant to avoid duplication.

[joel-costigliola]

I have one for eclipse and I know there is an eclipse plugin for Idea, I have given it a try in Idea 13 but it was not fully compatible with eclipse formatter preferences. Use it, this the best option we have here.
Since I rebase PR instead merging them, I will format the code à la Eclipse anyway.

Yes, one error message = one class, although I'm ok to reuse some like you did for ShouldBeAbsolutePath.

Another missing thing that I really care about : javadoc with code example(s) for every assertion API.
I know some assertion javadocs don't have code examples and it's a bad thing unless the assertion is obvious.
I'm adding javadoc code example from time to time when missing but for new assertions let's make this right in the first place.

Otherwise, the code seems fine so thanks for the work 👍

[joel-costigliola]

@fge do you plan to continue working on this issue in a near future ?

[fge]

Sorry for the delay, but yes I do. I am working on some stuff for myself at the moment, if you wish that this be done rapidly then I can dedicate my time to it!

Note about tests (meh, always strange to be talking about tests for a test framework): in order to verify the accuracy of assertions I write, I'd like to rely on memoryfs since it works relatively well, would that be an option?

[joel-costigliola]

I'm planning the 2.0 release in few weeks and Path assertions need definitely to be in 2.0, it would be nice if you could finish this within 2/3 weeks but if you don't that's ok, I'll finish the work, no problem.

I'm not against using memoryfs if it allows to to test things that would be really difficult with a real file system but I'm a little bit worried that some tests would pass with memoryfs and fail with a real file system. My opinion is then using the real file system as first option and memoryfs in second.
I'm all hear if you have additional arguments.

[fge]

Well, memoryfs allows to test things like, for instance, Files.isSameFile(); with a Windows filesystem you just cannot do that, whereas with an emulated Unix filesystem (which memoryfs does just fine) you can.

Among other things this would also allow to test the results of whether a given FileAttributeView is supported etc etc.

OK, resuming work on it. First things first, expect an amended commit, rebased on master, with your concerns addressed. I'll then add more tests.

Note: given the variety of filesystem implementations, some tests are going to be difficult to write at all; for instance, check that two Paths have the same content. If both support the creation of FileChannels then it is as easy as creating MappedByteBuffers out of them and check whether those are equals -- easy; if, however, the backing filesystem of a Path does not support the creation of a FileChannel, the "good old" method of reading byte arrays and comparing will be the way.

Well, I am probably getting ahead of myself at the moment. For now --> code.

[fge]

OK, updated with at least javadoc. However:

Another missing thing that I really care about : javadoc with code example(s) for every assertion API.

Where? In the AbstractAssert* implementation?

In the meantime, I'll continue writing assertions...

[joel-costigliola]

add code example

[joel-costigliola]

Thanks for taking the time to work on this.

I have added notes where javadoc code examples were missing.

If you have time (don't worry if you don't, I'll do it) I would like to see these assertions:

• hasParent
• hasNoParent
• startsWith
• endsWith
• hasRoot

[fge]

I will add code examples as requested.

As to assertions, the ones you mention are planned already; but there are a lot more I plan to add.

Before that however: do you agree with the added dependency? Also, do you agree about the base test code? Note that I am a TestNG user, so my JUnit code may turn out to be lacking.

And yeah, code style... I'd rather fix it in one go once everything is written, if you don't mind?

[joel-costigliola]

I would just get rid of PathsBaseTest_NoFileSystem, override setup calling PathsBaseTest#setup and initialising mock. Having a class PathsBaseTest_NoFileSystem with a initPath feels wrong to me.
IMO, if you need a base class for test that uses a mock path, its name should reflect that, maybe something like WithMockPathsBaseTest ...

[fge]

The problem is that for some assertions I'll need more than one path (example: test that two paths share the same filesystem; test that the target of a symlink is a given path; etc etc).

For tests that only use paths, I really want to avoid having to initialize a FileSystem. What is more, and that is a real problem to me, JUnit's @{Before,After}Class methods are static (unlike TestNG's), which means that I have to initialize/destroy a FileSystem after each test; that is expensive.

I'm not sure about how to tackle this problem, really. I do want to avoid having to create a FileSystem for each and every test, especially for classes which do not need to have one in the first place; and ideally I'd like to have the filesystem created and closed per test class not per test, and have the test class initialize it with the contents needed, not the base class.

This is a piece of cake with TestNG, not so with JUnit :(

[joel-costigliola]

I agree adding a test dependency.

for the code style, don't worry too much, I'll rebase your commits and take of format discrepancies.

more assertions ? like it ! :)

[fge]

I completely rebased the commits so that they look cleaner.

Note about reallyExists() vs exists(): I have chosen this naming even though another solution would have been to have a boolean argument telling whether to follow symlinks or not. Is that OK?

[joel-costigliola]

I find reallyExists() confusing, it makes me think that exists() is not really doing its job properly.
I suggest to use exists(boolean followSymlink) and exists() as a shortcut for exists(true) since by default links are followed in the JDK, cf Files#exists(Path path, LinkOption... options).
WDYT ?

Small change needed in javadoc comments (which are nice btw), you need to enclose code with <pre><code class='java'> and </code></pre> instead of <pre> and </pre>, if code is not java replace class='java' by the target language (ex: class='sh') - it uses highlight.js the hood.

[fge]

I find reallyExists() confusing, it makes me think that exists() is not really doing its job properly.
I suggest to use exists(boolean followSymlink) and exists() as a shortcut for exists(true) since by default links are followed in the JDK, cf Files#exists(Path path, LinkOption... options).
WDYT ?

OK, will do.

Small change needed in javadoc comments (which are nice btw), you need to enclose code with

 and 

instead of

 and 

, if code is not java replace class='java' by the target language (ex: class='sh') - it uses highlight.js the hood.

Nice addition! I should look at that for my own javadoc... Will do.

[fge]

Uh, pom is currently failing because of a missing mockito-core version dependency. Parent pom needs fixing?

[joel-costigliola]

raah, I have forgotten to push maven parent pom modifications ! :(
It's done now but you need to maven install parent pom locally (it's a SNAPSHOT - 1.3.2-SNAPSHOT). I have just released 1.3.2 release but it takes couple hours to be published in maven central.
Once available, I will use it for assertj-core pom.

sorry for the mess !

[joel-costigliola]

It should be fixed now as parent pom 1.3.2 is available.

[fge]

Confirmed, it fixes it! By the way, mockito 1.10.17 is out.

About javadoc: should I heavily comment Paths as well as AbstractPathAssert?

[fge]

Ah, yeah, also, what is the preferred format for error messages? Sometimes I see that there are messages preceded by a newline before "expected", sometimes not...

[fge]

OK, can you please carefully review the last assertion?

It is the first assertion which can throw an IOException; as such I modeled the tests etc on an equivalent FileAssert which can also fail due to an I/O error.

I have also noted that there are files which use tabs for indentation; most of them use spaces...

Meanwhile, writing other assertions!

(yeah, also, you may have seen that I use a @ClassRule when I do need a FileSystem to run assertions; my JUnit-fu is poor since I'm a TestNG user, and in TestNG, per-class test resources don't need to be static...)

[fge]

OK, another one to review carefully is .hasParent(). I have mimicked the equivalent File assertion, but this is far from being the equivalent of Path's .getParent().

[joel-costigliola]

Don't waste your time on Paths, I just want awesome javadoc on the API.
Add javadoc on Paths if you feel it's worth it.

Error messages are not consistent, I have started migrated them to start with a new line, use 2 spaces before displaying a value on a new line, a good example is ShouldBeEqualWithinOffset:

Expecting:
  <8.1>
to be close to:
  <8.0>
by less than <0.01> but difference was <0.1>.
(a difference of exactly <0.01> being considered valid)

I will update mockito in a future commit, I don't have specific reasons to use the latest version but it's always good to be up to date.

What is exactly the "last" assertion you want me to review carefully ?

[fge]

Don't waste your time on Paths, I just want awesome javadoc on the API.

OK, will do that.

Error messages are not consistent, I have started migrated them to start with a new line, use 2 spaces before displaying a value on a new line

OK, will do that.

What is exactly the "last" assertion you want me to review carefully ?

Uh yeah, I've rebased a lot so the track has been lost.

I'm talking about .has{,No}Parent() assertions. The thing is that assertions as they are currently written behave completely differently than Path's .getParent(); the latter will not attempt any normalization and/or resolution, the assertion does (.toRealPath() performs normalization, by the way, but .toAbsolutePath() does not!).

In a similar vein, I'm writing the assertion for .startsWith() at the moment and if I were to do it in a similar vein to .has{No,}Parent() then I'd .toRealPath() on both actual and the argument before checking the result.

Is that what you want?

[joel-costigliola]

My first thoughts would be that we should stick to the JDK behaviour to avoid confusing users, is it possible not to do any kind of normalization and/or resolution ?

[fge]

Well, it is possible, but then this is not what the equivalently named assertion in File does. The choice is yours.

[fge]

OK, I have transformed .has{,No}Parent() to behave like their relevant Path methods.

It may have surprising results though since with this modification, /home is not a parent of /home/foo/.. and /home/.. has a parent ;) And this is unlike what the equivalent assertions in Files do.

[joel-costigliola]

rename to existNoFollowLinks

[joel-costigliola]

@fge any update on this issue ? Do you want me to finish it ?

[fge]

Yes, please do.

Unfortunately, I've been very busy with other stuff lately... Sorry :(

Note however that the first commit is a proposed fix for issue #318 and that the code currently depends on this fix.

[joel-costigliola]

No problem, thanks for your work !

[joel-costigliola]

finally done, thanks @fge for the work !

[fge]

First of all, I am sorry for not having had the time to complete my branch to your expectations...

And, uh, next question, in which version will those tests show up? ;)

[joel-costigliola]

2.0, planned in a few weeks.