Windows Unit Tests All Fixed (fix #402) #404
Conversation
|
@advayDev1 - ignore for now, still in progress |
brunobowden
force-pushed
the
update
branch
8 times, most recently
from
c74d3ef
to
01bbd50
Compare
brunobowden
changed the title
brunobowden
force-pushed
the
update
branch
2 times, most recently
from
c54d6e7
to
948d20e
Compare
|
@advayDev1 and @gocursor - I've gone through and fixed up all the unit tests on Windows. A small number (~4) were disabled with TODOs. This was complicated by not having a Windows machine ;-). @cursor, if you could look at trying to fix some of the disabled Windows tasks marked with TODOs, that would be helpful, in particular it needs a substitute for running 'echo' on the command line. @advayDev1 - I still have one weird failure locally but it doesn't happen on the build bots. Very happy to see this all working:
|
brunobowden
changed the title
| @@ -99,6 +135,29 @@ class Utils { | |||
| } | |||
| } | |||
|
|
|||
| // Same as File.separator but can mocked using fakeOSName | |||
There was a problem hiding this comment.
s/mocked/faked/ below as well
|
Awesome - really nice work! Can you add to CONTRIBUTING.md style guide the canonical way to construct a file paths such that they work across platforms and across fakes OSes? I see sometimes you using new File(parent, child).absolutePath and sometime concating with the Utils separator. |
brunobowden
force-pushed
the
update
branch
4 times, most recently
from
18e5b2a
to
9068d90
Compare
|
@advayDev1 - PTAL, this is all fixed up, including the local failure that @gocursor also saw. It should be ready for a LGTM now. |
| // https://github.com/j2objc-contrib/j2objc-gradle/issues/422 | ||
| String executableCmd = 'echo' | ||
| if (Utils.isWindowsNoFake()) { | ||
| // executableCmd = '????' |
There was a problem hiding this comment.
looks like your new echo solution is needed here too?
There was a problem hiding this comment.
Good point. Fixed that up too.
|
LGTM. see 1 comment in the code and also please respond to my comment above:
|
brunobowden
force-pushed
the
update
branch
2 times, most recently
from
4ffc6ee
to
ab9fdc4
Compare
|
I'll add the section to CONTRIBUTING in my next PR. Just waiting on builds and then I'll merge. |
|
sg |
Major: - Fix j2objc-contrib#402 - Windows 37 unit tests failures - Fix j2objc-contrib#422 - echo command fails on Windows - Disabled only failing test on Windows: j2objc-contrib#374 Minor: - Fake OS specific file and path separators - Explicit setFakeOS methods, e.g. setFakeOSWindows() - Default to using native OS rather than fake OS for most unit tests - Small number of unit tests use fakeOSName - ‘inTestMustFakeOS’ setting removed
brunobowden
changed the title
Windows Unit Tests All Fixed (fix #402)
Major:
Minor: