add appveyor.yml #1381

Merged
merged 7 commits into from Aug 10, 2016

Projects

None yet

2 participants

@lukechampine
Member
lukechampine commented Jul 26, 2016 edited

This should give us Windows builds. I'm still tweaking the configuration. Ideally, it should match Travis (build all PR commits + merge commit)
This is complicated somewhat by the fact that Windows does not natively support Makefiles, so we must make do with the go command.

@lukechampine
Member

These tests fail on Windows:

-- FAIL: TestNew (0.00s)
    contractor_test.go:91: expected permissions error, got invalid character '\x01' looking for beginning of value
--- FAIL: TestNew (0.00s)
    hostdb_test.go:69: expected permissions error, got <nil>

should be an easy fix

@lukechampine
Member
lukechampine commented Jul 26, 2016 edited

I decided to simply remove the offending tests in my previous comment because they didn't add much useful coverage and I couldn't find an easy way to fix them.

Further failures:

--- FAIL: TestReloading (0.42s)
    server_test.go:45: read C:\Users\appveyor\AppData\Local\Temp\1\SiaTesting\api\TestReloading\consensus\consensus.db.lock: The process cannot access the file because another process has locked a portion of the file.

--- FAIL: TestStorageFolderUsage (3.19s)
    storagefolders_smoke_test.go:924: unexpected number of files in the manager directory

--- FAIL: TestRenterPaths (0.53s)
    persist_test.go:289: Bad walk string: expected foo/bar/baz.siafoo/bar.siafoo.sia, got foo\bar\baz.siafoo\bar.siafoo.sia

--- FAIL: TestOpenDatabase (0.00s)
    boltdb_test.go:102: calling OpenDatabase on a new database failed for metadata {1sadf23 12253}, filename  ; error was open C:\Users\appveyor\AppData\Local\Temp\1\SiaTesting\persist\TestOpenNewDatabase\ : Access is denied.

--- FAIL: TestErrPermissionOpenDatabase (69.74s)
    boltdb_test.go:228: OpenDatabase failed to return expected error when called on a database with the wrong permissions (577 instead of >= 0600);
         wanted:    open C:\Users\appveyor\AppData\Local\Temp\1\SiaTesting\persist\TestErrPermissionOpenDatabase\Fake Filename: permission denied
         got:       timeout
    boltdb_test.go:232: remove C:\Users\appveyor\AppData\Local\Temp\1\SiaTesting\persist\TestErrPermissionOpenDatabase\Fake Filename: The process cannot access the file because it is being used by another process.

Seems like they mostly stems from Windows' different file handle semantics. The permissions masking is different, and files must be closed before they can be removed.

@lukechampine
Member

sorry @mnsl, Windows doesn't seem to like ¯|_(ツ)_|¯ as a filename 😑

@DavidVorick
Member

I have definitely seen users reporting some of these errors, very glad to see them being caught by testing.

@lukechampine
Member

Four errors left:

--- FAIL: TestReloading (0.42s)
    server_test.go:45: read C:\Users\appveyor\AppData\Local\Temp\1\SiaTesting\api\TestReloading\consensus\consensus.db.lock: The process cannot access the file because another process has locked a portion of the file.

--- FAIL: TestBlankStorageObligation (0.81s)
    storageobligations_smoke_test.go:171: host should have 0 contracts, the contracts were all completed: 1

--- PASS: TestIntegrationAutoRescan (0.44s)
A lock was held for too long, id '25'. Call stack:
    File: 'C:/GOPATH/src/github.com/NebulousLabs/Sia/modules/host/storageobligations.go:674'
    File: 'C:/go/src/runtime/asm_amd64.s:1998'
    File: ':0'
panic: Critical error: negative currency not allowed
Please submit a bug report here: https://github.com/NebulousLabs/Sia/issues
goroutine 519 [running]:
panic(0xa9c720, 0xc0820021b0)
    C:/go/src/runtime/panic.go:481 +0x40d
github.com/NebulousLabs/Sia/build.Critical(0xc082364618, 0x1, 0x1)
    C:/GOPATH/src/github.com/NebulousLabs/Sia/build/critical.go:13 +0x107
github.com/NebulousLabs/Sia/types.Currency.Sub(0x0, 0x0, 0x0, 0x0, 0x0, 0xc08244e900, 0x2, 0x5, 0xc0821c8c00, 0x0, ...)
    C:/GOPATH/src/github.com/NebulousLabs/Sia/types/currency.go:167 +0x1f5
github.com/NebulousLabs/Sia/modules/host.(*Host).removeStorageObligation(0xc082530f00, 0xc082487e00, 0x1, 0x4, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, ...)
    C:/GOPATH/src/github.com/NebulousLabs/Sia/modules/host/storageobligations.go:538 +0x12e4
github.com/NebulousLabs/Sia/modules/host.(*Host).threadedHandleActionItem(0xc082530f00, 0x97c94a50855ae4c4, 0xbeade34eb78ceb6c, 0xbcfbe40fbd70e9d6, 0x6093a3f34d282393, 0xc0824d6b40)
    C:/GOPATH/src/github.com/NebulousLabs/Sia/modules/host/storageobligations.go:675 +0x1774
created by github.com/NebulousLabs/Sia/modules/host.(*Host).ProcessConsensusChange
    C:/GOPATH/src/github.com/NebulousLabs/Sia/modules/host/update.go:322 +0x4ad

--- FAIL: TestStorageFolderUsage (3.58s)
    storagefolders_smoke_test.go:924: unexpected number of files in the manager directory

The second one is ND, have only seen it once. I can probably handle the TestReloading error, but @DavidVorick may need to investigate the host-specific ones.

@DavidVorick
Member

now that the host has flushing we should be able to clean up the smoke test pretty easily.

The third error I think is just a combination of safelocks in the host and appveyor being slow. We can probably pull the safelocks out of the host at this point - I found the deadlock that I was searching for.

The final one is pretty confusing, would help if we could print all the files that it is finding. I'm surprised it's not having trouble with the symlinks? Is it running as admin?

lukechampine added some commits Jul 26, 2016
@lukechampine lukechampine add appveyor.yml 88e3366
@lukechampine lukechampine fix various Windows test failures 4b9afd6
@lukechampine lukechampine remove some filenames that Windows can't handle f8b7ed5
@lukechampine lukechampine close serverTester in TestReloading
16f7d81
@lukechampine lukechampine remove OS-specific error check 6ee3e79
@lukechampine lukechampine count lock file in TestStorageFolderUsage
7f2a0e4
@lukechampine
Member

this should be ready. We probably want to merge it before the host PR so that we can confirm that the changes work on Windows.

@DavidVorick DavidVorick commented on the diff Aug 9, 2016
api/renter_test.go
@@ -453,8 +453,8 @@ func TestRenterLoadNonexistent(t *testing.T) {
uploadValues := url.Values{}
uploadValues.Set("source", fakepath)
err = st.stdPostAPI("/renter/upload/dne", uploadValues)
- if err == nil || !strings.HasSuffix(err.Error(), "no such file or directory") {
- t.Errorf("expected error to end with 'no such file or directory; got %v", err)
+ if err == nil {
+ t.Errorf("expected error when uploading nonexistent file")
@DavidVorick
DavidVorick Aug 9, 2016 Member

hmm. You can't os.DNE?

@lukechampine
lukechampine Aug 9, 2016 Member

sadly no, because the error is converted to a string and back.

@DavidVorick DavidVorick commented on the diff Aug 9, 2016
modules/renter/contractor/contractor_test.go
@@ -78,18 +78,6 @@ func TestNew(t *testing.T) {
if _, ok := err.(*json.SyntaxError); !ok {
t.Fatalf("expected invalid json, got %v", err)
}
-
- // Corrupted logfile.
- os.RemoveAll(filepath.Join(dir, "contractor.log"))
- f, err := os.OpenFile(filepath.Join(dir, "contractor.log"), os.O_CREATE, 0000)
- if err != nil {
- t.Fatal(err)
- }
- defer f.Close()
- _, err = New(stub, stub, stub, stub, dir)
- if !os.IsPermission(err) {
- t.Fatalf("expected permissions error, got %v", err)
- }
@DavidVorick
DavidVorick Aug 9, 2016 Member

why does this code need to be removed?

@lukechampine
lukechampine Aug 9, 2016 Member

I couldn't find a way to create a file with bad permissions on Windows. On Linux I would use Umask, but there doesn't seem to be a Windows equivalent of that syscall.

@DavidVorick DavidVorick commented on the diff Aug 9, 2016
modules/renter/persist_test.go
@@ -285,7 +285,7 @@ func TestRenterPaths(t *testing.T) {
})
// walk will descend into foo/bar/, reading baz, bar, and finally foo
expWalkStr := (f3.name + ".sia") + (f2.name + ".sia") + (f1.name + ".sia")
- if walkStr != expWalkStr {
+ if filepath.ToSlash(walkStr) != expWalkStr {
@DavidVorick
DavidVorick Aug 9, 2016 Member

what's the difference here?

@lukechampine
lukechampine Aug 9, 2016 Member

the (sia) filenames contain Unix-style slashes, but walking the directory will produce OS-specific slashes.

@DavidVorick DavidVorick commented on the diff Aug 9, 2016
persist/boltdb_test.go
"你好好q wgc好",
"\xF0\x9F\x99\x8A",
"",
"",
"$HOME",
",.;'[]-=",
- "A:",
@DavidVorick
DavidVorick Aug 9, 2016 Member

lol Windows

@DavidVorick DavidVorick and 1 other commented on an outdated diff Aug 9, 2016
persist/boltdb_test.go
@@ -196,9 +202,8 @@ func TestOpenDatabase(t *testing.T) {
// with the wrong filemode (< 0600), which should result in an os.ErrPermission
// error.
func TestErrPermissionOpenDatabase(t *testing.T) {
- if testing.Short() {
- t.SkipNow()
- }
+ t.Skip("can't reproduce on Windows")
@DavidVorick
DavidVorick Aug 9, 2016 Member

earlier in the test the directory is created with permission '0700', perhaps changing that will produce the correct results?

@lukechampine
lukechampine Aug 9, 2016 Member

I think I tweaked it a few times and was unable to get it to work.
Probably better to wrap it in a runtime.GOOS != "windows" rather than disabling it outright, though

@lukechampine
lukechampine Aug 9, 2016 edited Member

actually, I think I know what's happening here. Bolt uses os.OpenFile with the os.O_CREATE flag, meaning it will create the file if it does not exist. But it does not use this flag:

O_EXCL int = syscall.O_EXCL   // used with O_CREATE, file must not exist

My guess if that if you don't use O_EXCL on Windows, it will simply overwrite the badly-permissioned file with a properly-permissioned file. This would explain the other permissions test failure as well, since it's the same deal: we use O_CREATE when opening log files, but not O_EXCL.

EDIT: well, that sort-of worked. You get an error if you try to open a file with bad permissions, but you also get an error if you try to reopen a file at all. Back to the drawing board.

@lukechampine
lukechampine Aug 9, 2016 Member

I think you were right about the directory. Files on Windows seem to inherit permissions from their parent directory. However, if we restrict permissions on the folder (i.e making it read-only), then we can no longer create files inside it!

I'm just gonna throw an OS guard around it and call it a day.

@lukechampine lukechampine only skip permissions test on Windows
778f9f2
@DavidVorick DavidVorick merged commit c79980f into master Aug 10, 2016

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@lukechampine lukechampine deleted the appveyor branch Aug 16, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment