Update tests to pass on linux

[JamesRamm]

This PR updates the tests to pass on linux. I have not been able to check these tests are still ok on windows yet, so this would need double checking first.

It consists of 3 main changes:

• Replace all backslashes with forward slashes. Windows fully supports forward slashes as directory separators, so shouldn't be a problem (although Im not sure if tuflow requires backslashes on windows or not).

• Added a utility function to make all the fake absolute paths platform specific. (Basically just checks the platform and if it is linux, replaces the drive letter with a forward slash)

• The above resulted in 1 failing test so I had to change some code in filetools (setFinalFolder) which stripped the preceding '/' on linux paths. It should do exactly the same thing as previous code (?)

Hopefully this is a first small step toward ensuring x platform support.

[JamesRamm]

Just a note to say; travis is failing because of the integration tests (which this PR does not address), the log shows the unit tests to be passing now.

[duncan-r]

Thanks @JamesRamm

I've had a look through this and it's causing some issues when running the tests on windows. The failures are to do with the difference between the the way that PathHolder stores the path variables and the way that the new utils.fakeAbsPath() converts the path. I.e. the path holder converts to os format, but the fakeAbsPath doesn't convert absolute paths, only non-absolute paths. E.g.

===================================================================
FAIL: test_absolutePath (tests.test_filetools.PathHolderTests)
Test that the absolute path is returned correctly.
----------------------------------------------------------------------
Traceback (most recent call last):
  File "c:\users\duncan\documents\program\python\ship\tests\test_filetools.py", line 67, in test_absolutePath
    self.assertEqual(self.fake_abs_path, abs_path, 'absolutePath() fail')
AssertionError: 'c:/first/second/third/fourth/TuflowFile.txt' != 'c:\\first\\second\\third\\fourth\\TuflowFile.txt'
- c:/first/second/third/fourth/TuflowFile.txt
?   ^     ^      ^     ^      ^
+ c:\first\second\third\fourth\TuflowFile.txt
?   ^     ^      ^     ^      ^
 : absolutePath() fail

I think this one can probably be solved by ensuring that the utils function updates the absolute path, in the same manner as the other check. E.g.

def fakeAbsPath(path):
    '''
    Make a pretend filepath 'absolute' for the current os.
    Will massage absolute paths for the wrong OS to abs paths
    for current os
    '''
    if os.path.isabs(path):
        # --> Do the same replace here  <--
        if os.name == 'posix':
            newpath = path.replace('\\', os.sep)
        else:
            newpath = path.replace('/', os.sep)
    elif os.name == 'posix':
        # We may have been given a windows path, so remove the drive and
        # make it root dir
        newpath = path.replace('c:', '').replace('\\', os.sep)
        if not newpath.startswith('/'):
            newpath = '/{}'.format(newpath)
    else:
        newpath = 'c:{}{}'.format(os.sep, path)

    return newpath

We're also having the same issues with the relative paths. So another function in utils for handling that will be needed to:

def fakeRelPath(path):
    if os.name == 'posix':
        newpath = path.replace('\\', os.sep)
    else:
        newpath = path.replace('/', os.sep)

    return newpath

Which can used in the same way: fakeRelPath('../fifth')

I realise that this is all a bit tricky for you to check on a linux env, so I'm happy to pull it down, make the changes and get it merged in if you want?

However, I think this all points to a bit of an issue in the way the PathHolder class stores the path information. I think perhaps that we need to consider checking what the os is at the start of the load, storing that and handling the way paths are stored and updated based on that the whole way through. We should probably also consider whether to allow the user to set what type of paths they would like to use when writing out the files. It may be that you run it on linux, but want to write paths out as windows, or visa-versa (which could be achieved by setting the root variable. Also, tuflow doesn't care whether you use forward or back slashes in relative paths, but the user may have a preference. So we should probably allow that preference. I will have a bit of a think about this.

Don't worry about the integration tests. They're pretty rough, so I'm happy to make sure that they're setup properly and everything is passing once this work is complete. I'm thinking it might be a good idea to create a 'pathfixes' branch or something to do all of this on?

Let me know if you have any thoughts.

[duncan-r]

I've had a bit of a think about this and have some ideas:

• It is probably not a massive amount more work to make it so the setting of the file path formats can be specified by the user. I.e. they can provide a new root and set the paths to be posix or windows. This would make it pretty simple to have a script that can convert paths from linux to windows or visa-versa by just changing the root variable - and potentially an os_type variable in the TuflowModel.
• We could probably just say that if files are being treated as windows/linux we will write them out with windows/linux style path separators. I've always tried quite hard to avoid changing the the original style and formatting of the files that are read in, but I think this is a small concession to make life a lot easier.
• After doing this we could then build the paths in the PathHolder with a separator defined by the currently set os option. In this way we wouldn't be using the os.path functions and could therefore create unix paths on windows or the other way around.
• It would make cross platform testing easier as we are guaranteed to get the path style we expect and we can simply test whether setting the os type and root works in there.

[JamesRamm]

With the exception of setFinalFolder I don't think PathHolder/filetools.py would cause any x-platform issues as it is pretty much using standard lib functions. There are a couple of functions which seem to be to get around using is_file or is_dir in order to cater for fake path?

One solution to fix the tests would be:

• Have a module-level global set which will set the 'drive' for abs path. e.g.:

  DRIVE_PREFIX = '/'
  if os.name != 'posix':
  DRIVE_PREFIX = 'c:'

• Then use os.path.join to construct all paths:

  fake_abs_path = os.path.join(DRIVE_PREFIX, 'first', 'second', 'third')
  fake_rel_path = os.path.join('..', 'first')

  This will probably make the tests pass as-is without any changes to PathHolder.

In reply to your points above - In general, I think it would be safe to assume that the machine SHIP is being run on is the same machine that tuflow would be run on? In which case we should always throw errors for badly formatted paths. We can then simply safely use standard library functions without worrying about the OS (with the exception of the setFinalFolder function, which can be changed to use os functions anyway). I think it would be a more unusual situation for a user to want to run SHIP on linux but then tuflow on windows or vice versa. Even if that did occur; they obviously have access to the other machine so can easily run whatever SHIP stuff they need to do on the same machine (or at least a machine with same OS). Then we can also easily write in validation checks for the file being read in (i.e. do those files actually exist).

I think a truly robust solution will require changes to both filetools and the unit tests. It is generally more common to have small test data (for a relevant example see the gdal tests) or generate files on the fly for unit tests than use fake paths. We probably want (should) expect SHIP to fail if paths dont exist / are malformed on the current OS.

A problem then for generating files and then passing the relative path is that it needs to be relative to something. Tuflow expects all paths to be relative to the tcf file. Essentially, to properly handle relative paths in PathHolder, it needs to know what it is relative to. I.e I think PathHolder needs some knowledge of the absolute path to the TCF file.

[JamesRamm]

PS; it is probably a good idea to move this discussion to an issue...