Try to make tests run on Windows #41

hustf · 2015-10-17T20:38:17Z

This effort stranded on

ccall(:dup, Int32, (Int32,) fd) on GZip.jl:278
could not load symbol "dup" on Windows, not on OSX.

Libz is planned to replace Gzip. This is submitted in case this is picked up again for some reason.

simonster · 2015-10-17T21:04:26Z

Travis doesn't support Windows.

kmsquire · 2015-10-18T02:13:19Z

@hustf, it seems that travis doesn't like tabs in the .travis.yml.

ranjanan · 2016-07-27T11:25:19Z

Bump!

Can this PR be rebased? I noticed @tkelman enabled Appveyor, so can we enable it for this PR and then tag a new release?

@bmharsha, is this a fix for #48 ?

hustf · 2016-07-27T18:06:43Z

I´ll do that, and see if #48 is affected before and after.

hustf · 2016-07-29T19:24:35Z

The first test (line 49) is not mean to be run if gunzip.exe is not present. But is there any point in keeping it around for other OS?

It runs the standalone program gunzip.exe with command line options. On Windows, it could be identified using 'where.exe gunzip.exe', if it happens to be on PATH. Or we could use something like JULIA_HOME..\Git\Gunzip.exe.

The .exe file was included with Julia until 0.4.0 rc4 and possibly a bit longer. Now, it's not included anymore.
The library that is actually used ("zlib1" on Windows) is independent of Gunzip.exe. So this is an irrelevant test as far as I can see. I suppose this is the case for other OSs as well?

tkelman · 2016-07-30T01:48:52Z

I think the package only uses the library, but the tests use the executable for verification?

tkelman · 2016-07-30T01:50:53Z

on 0.5, you should be able to run busybox gunzip on windows but that won't work elsewhere if busybox is not installed. the git paths changed between 0.4.0 and 0.4.1, they're kind of internal programs but okay for testing.

hustf · 2016-07-30T21:06:22Z

Thanks for the info about busybox.
I kept the gunzip test on unix.
Line 57; gzfdio or gzdopen. This is not found in the windows library. This is not a test of the library, so I find it unreasonable to signal a failed check here.

kmsquire · 2016-07-31T04:26:29Z

test/runtests.jl

+        close(gzfile)
+        close(raw_file)
+        @test data == data4
+	end 


Looks like a tab snuck in here...

kmsquire · 2016-07-31T04:30:46Z

There's a merge conflict, currently, so AppVeyor can't run. Can you rebase on master?

hustf · 2016-07-31T08:48:19Z

kmsquire, thanks for input, tabs are replaced with spaces now.
Note to self, next time:

Rebase forked master.
Rebase forked branch with forked master
Make sure everything is totally up to date
Check out branch, work, commit, git push origin

tkelman · 2016-07-31T10:38:23Z

A bunch of unrelated commits are now in here. Could you rebase again, check that your local branch only has your commits on top of the current master, then be sure to force push over your fork's branch?

hustf · 2016-07-31T11:19:32Z

Tony, that´s right. Like the January 9 committs are now referred here. That´s an uninteded result of rebasing my fork master, then rebasing my brach fixDep. But I´m afraid I don´t know how to force push. It would be easier for me to delete this and start over with a new fork, unless you have the exact commands in front of you?

tkelman · 2016-07-31T11:54:50Z

push with the --force flag

- Added Compat for 0.3.11, changed to 0.4 Union syntax line 75 - Revoked travis.yml; now with git config --global core.autocrlf input. - Added appveyor.yml; copy of Compat appveour replacing package name. - Removed two tests on Windows, shortened try..catch to intended error messages only. - Tab changes - Removed line end spaces

hustf · 2016-07-31T15:11:34Z

I guess that is it now....? Merge?

tkelman · 2016-07-31T15:15:55Z

test/runtests.jl

@@ -91,7 +93,9 @@ try
    NEW = GZip.GZLIB_VERSION > "1.2.3.9"
    pos = position(gzfile)
    NEW && (pos2 = position(gzfile,true))
-    @test_throws ErrorException seek(gzfile, 100)   # can't seek backwards on write
+    try 
+        @test_throws ErrorException seek(gzfile, 100)   # can't seek backwards on write


why is this in a try?

It used to be in a large try block from line 29/31. Line 97 actually throws an error; the test checks that it throws the correct error. Without a try, the code execution would halt. Correct me if I'm wrong.

@test_throws shouldn't halt if the right type of exception gets thrown

and try finally without a catch will also terminate on an error, just with the cleanup tasks performed

When commenting out 'catch' line 96 and replacing 98 with 'println("l98") the result is:

julia> include("runtests.jl") ERROR: LoadError: test failed: seek(gzfile,100) did not throw ErrorException in expression: seek(gzfile,100) in error at error.jl:21 while loading C:\Users\F\Documents\GitHub\GZip.jl\test\runtests.jl, in expression starting on line 97

This in Julia 0.4.6, Windows 10.

so it didn't error at all, but it was supposed to. is this a bug or platform IO difference? I'm not sure.

That is right, I did not notice that at first. But we see in other places that there are differences in the library.

tkelman · 2016-08-05T01:08:45Z

test/runtests.jl

@@ -91,7 +93,9 @@ try
    NEW = GZip.GZLIB_VERSION > "1.2.3.9"
    pos = position(gzfile)
    NEW && (pos2 = position(gzfile,true))
-    @test_throws ErrorException seek(gzfile, 100)   # can't seek backwards on write
+    try


still don't think this should be a try catch. it could be disabled on windows if it's broken there, but do we know why it's broken?

I think only the error message is broken.
seek(gzfile, 100) returns true in all cases, whether the current position can be changed or not.
To test the behaviour on Windows, we could check that the position remains unchanged after running seek(gzfile,100) or seek(gzfile, 1000000)
A more thorough approach might be to actually throw an error in Windows too, around Gzip.jl:328.

is it a different exception type on windows then? or no exception at all?

… Windows only.

SimonDanisch · 2016-08-08T18:18:39Z

there are a typos in:
https://github.com/hustf/GZip.jl/blob/03ea8ad49fe0fdfaf7e7c2ff213fc7ace9846cec/appveyor.yml#L41
https://github.com/hustf/GZip.jl/blob/03ea8ad49fe0fdfaf7e7c2ff213fc7ace9846cec/appveyor.yml#L38
Gzip --> GZip

hustf · 2016-08-08T18:41:59Z

Simon, thanks, but appveyor was put in this PR and also merged separately while this was still open. I have rebased to make the history as clear as possible. There are many improvements which could be made here, but I believe in making short and uncomplicated pull requests (in cases like this, that is!).

kmsquire · 2019-11-26T21:48:12Z

Windows tests have been working for a while now.

hustf mentioned this pull request Oct 17, 2015

Add Appveyor support - Windows testing #40

Closed

tkelman mentioned this pull request Dec 7, 2015

enable AppVeyor #44

Merged

hustf mentioned this pull request Jul 27, 2016

Pkg.test("GZip") fails on Windows #48

Closed

kmsquire reviewed Jul 31, 2016
View reviewed changes

test/runtests.jl

close(gzfile)

close(raw_file)

@test data == data4

end

Copy link

Contributor

kmsquire Jul 31, 2016

Choose a reason for hiding this comment

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

Looks like a tab snuck in here...

hustf added 2 commits July 31, 2016 14:53

Removed ";" for suppressed terminal output.

018bf0e

hustf force-pushed the fixDep branch from 1eebfcf to 19321c5 Compare July 31, 2016 15:06

tkelman reviewed Jul 31, 2016
View reviewed changes

Removed ; at line 68.

6f431bd

tkelman reviewed Aug 5, 2016
View reviewed changes

Disabled checking of error message on backwards seek in a write file.…

03ea8ad

… Windows only.

SimonDanisch mentioned this pull request Aug 8, 2016

Tag a version #57

Closed

kmsquire closed this Nov 26, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Try to make tests run on Windows #41

Try to make tests run on Windows #41

hustf commented Oct 17, 2015

simonster commented Oct 17, 2015

kmsquire commented Oct 18, 2015

ranjanan commented Jul 27, 2016

hustf commented Jul 27, 2016

hustf commented Jul 29, 2016

tkelman commented Jul 30, 2016

tkelman commented Jul 30, 2016

hustf commented Jul 30, 2016

kmsquire Jul 31, 2016

kmsquire commented Jul 31, 2016

hustf commented Jul 31, 2016

tkelman commented Jul 31, 2016

hustf commented Jul 31, 2016

tkelman commented Jul 31, 2016

hustf commented Jul 31, 2016

tkelman Jul 31, 2016

hustf Jul 31, 2016

tkelman Jul 31, 2016

tkelman Jul 31, 2016

hustf Aug 4, 2016

tkelman Aug 4, 2016

hustf Aug 4, 2016

tkelman Aug 5, 2016

hustf Aug 5, 2016

tkelman Aug 8, 2016

SimonDanisch commented Aug 8, 2016

hustf commented Aug 8, 2016 •

edited

Loading

kmsquire commented Nov 26, 2019

Try to make tests run on Windows #41

Try to make tests run on Windows #41

Conversation

hustf commented Oct 17, 2015

simonster commented Oct 17, 2015

kmsquire commented Oct 18, 2015

ranjanan commented Jul 27, 2016

hustf commented Jul 27, 2016

hustf commented Jul 29, 2016

tkelman commented Jul 30, 2016

tkelman commented Jul 30, 2016

hustf commented Jul 30, 2016

Choose a reason for hiding this comment

kmsquire commented Jul 31, 2016

hustf commented Jul 31, 2016

tkelman commented Jul 31, 2016

hustf commented Jul 31, 2016

tkelman commented Jul 31, 2016

hustf commented Jul 31, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

SimonDanisch commented Aug 8, 2016

hustf commented Aug 8, 2016 • edited Loading

kmsquire commented Nov 26, 2019

hustf commented Aug 8, 2016 •

edited

Loading