Skip to content

Conversation

@codeandcodes
Copy link

Per https://issues.apache.org/jira/browse/SSHD-605, these unit tests test the RootedFileSystemProvider (mirroring the API) to validate that FS actions taken outside of RootedFileSystem#getRoot() throw an InvalidPathException.

@codeandcodes
Copy link
Author

I've also confirmed that all tests pass with the new changes committed by @lgoldstein.

SSHD-605-RootedFileSystemProviderTest.txt

*/
public class AssertableFile extends BaseTestSupport {

public AssertableFile() { super(); }
Copy link
Contributor

@lgoldstein lgoldstein Apr 15, 2016

Choose a reason for hiding this comment

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

Please move superclass call to separate line

@lgoldstein
Copy link
Contributor

Very nice - a few minor issues to fix - once you do that and re-post I will be happy to merge your contribution into the master branch.


public static Boolean isNonEmpty(byte [] bytes) {
Boolean cond = bytes != null && bytes.length > 0;
assertTrue("bytes are non empty", cond);
Copy link
Contributor

Choose a reason for hiding this comment

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

  1. Please use byte[] (no space)
  2. Please use org.apache.sshd.common.util.NumberUtils.isEmpty(bytes)

@codeandcodes
Copy link
Author

Thanks for the quick review Lyor. Will post up an update tomorrow.

@codeandcodes
Copy link
Author

@lgoldstein I pushed up fixes per your comments. Tested with backing out the the fix and found the fail(...) messages were very helpful. Thanks for that suggestion.

@lgoldstein
Copy link
Contributor

lgoldstein commented Apr 15, 2016

Committed and pushed to the master branch - congratulations and thanks for your contribution. A few minor issues for future reference:

  1. You forgot to convert the Boolean(s) to boolean(s) - I did it this time
  2. The spacing and indentation were wrong - fixed that
  3. Copyright notice was missing - also fixed that

In the future, please run our mvn clean install as it applies the checkstyle and rat plugins as well. In general, we require the license/copyright notice at the top of the file (see other files in the project for the content). As far as style goes - we use

  • 4 spaces (no tabs)
  • no trailing whitespace
  • specific import order (java/javax/org/com - which is the default).
  • no unused imports
  • no static imports

These are just the main ones - when in doubt, look at other code and copy the style.All these can be easily configured in Eclipse and applied as code style as well as automatic save actions.

@lgoldstein
Copy link
Contributor

Please close the request since merged into main branch

@codeandcodes
Copy link
Author

Thanks @lgoldstein will keep in mind for future reference.

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.

2 participants