Merge pull request #2353 from nspcc-dev/fix-win-tests

*: fix windows tests
2022-02-10 21:57:10 +03:00 · 2022-02-10 21:57:10 +03:00 · acd348c13f
commit acd348c13f
parent 32a7e97a87 0760e5486b
8 changed files with 164 additions and 53 deletions
--- a/.github/workflows/publish_to_dockerhub.yml
+++ b/.github/workflows/publish_to_dockerhub.yml
@ -128,9 +128,6 @@ jobs:
        run: make test
  publish_wsc:
    # Ensure test job passes before pushing image.
-    # TODO: currently test_wsc job is failing, so we have `always()` condition.
-    # After #2269 and #2268 this condition should be removed.
-    if: ${{ always() }}
    needs: tests_wsc
    name: Publish WindowsServerCore-based image to DockerHub
    runs-on: windows-2022
@ -141,6 +138,9 @@ jobs:
          # Allows to fetch all history for all branches and tags. Need this for proper versioning.
          fetch-depth: 0

+      - name: Show docker images
+        run: docker images
+
      - name: Build image
        run: make image-wsc

--- a/Dockerfile.wsc
+++ b/Dockerfile.wsc
@ -1,5 +1,25 @@
 # Builder image
-FROM golang:windowsservercore-ltsc2022 as builder
+FROM mcr.microsoft.com/windows/servercore:ltsc2022 as builder
+
+SHELL ["powershell", "-Command", "$ErrorActionPreference = 'Stop';", "$ProgressPreference = 'SilentlyContinue';"]
+
+ENV GIT_VERSION=2.23.0
+
+ENV GIT_TAG=v2.23.0.windows.1
+
+ENV GIT_DOWNLOAD_URL=https://github.com/git-for-windows/git/releases/download/v2.23.0.windows.1/MinGit-2.23.0-64-bit.zip
+
+ENV GIT_DOWNLOAD_SHA256=8f65208f92c0b4c3ae4c0cf02d4b5f6791d539cd1a07b2df62b7116467724735
+
+RUN Write-Host ('Downloading {0} ...' -f $env:GIT_DOWNLOAD_URL); 	[Net.ServicePointManager]::SecurityProtocol = [Net.SecurityProtocolType]::Tls12; 	Invoke-WebRequest -Uri $env:GIT_DOWNLOAD_URL -OutFile 'git.zip'; 		Write-Host ('Verifying sha256 ({0}) ...' -f $env:GIT_DOWNLOAD_SHA256); 	if ((Get-FileHash git.zip -Algorithm sha256).Hash -ne $env:GIT_DOWNLOAD_SHA256) { 		Write-Host 'FAILED!'; 		exit 1; 	}; 		Write-Host 'Expanding ...'; 	Expand-Archive -Path git.zip -DestinationPath C:\git\.; 		Write-Host 'Removing ...'; 	Remove-Item git.zip -Force; 		Write-Host 'Updating PATH ...'; 	$env:PATH = 'C:\git\cmd;C:\git\mingw64\bin;C:\git\usr\bin;' + $env:PATH; 	[Environment]::SetEnvironmentVariable('PATH', $env:PATH, [EnvironmentVariableTarget]::Machine); 		Write-Host 'Verifying install ("git version") ...'; 	git version; 		Write-Host 'Complete.';
+
+ENV GOPATH=C:\\go
+
+RUN $newPath = ('{0}\bin;C:\Program Files\Go\bin;{1}' -f $env:GOPATH, $env:PATH); 	Write-Host ('Updating PATH: {0}' -f $newPath); 	[Environment]::SetEnvironmentVariable('PATH', $newPath, [EnvironmentVariableTarget]::Machine);
+
+ENV GOLANG_VERSION=1.17.6
+
+RUN $url = 'https://dl.google.com/go/go1.17.6.windows-amd64.zip'; 	Write-Host ('Downloading {0} ...' -f $url); 	[Net.ServicePointManager]::SecurityProtocol = [Net.SecurityProtocolType]::Tls12; 	Invoke-WebRequest -Uri $url -OutFile 'go.zip'; 		$sha256 = '5bf8f87aec7edfc08e6bc845f1c30dba6de32b863f89ae46553ff4bbcc1d4954'; 	Write-Host ('Verifying sha256 ({0}) ...' -f $sha256); 	if ((Get-FileHash go.zip -Algorithm sha256).Hash -ne $sha256) { 		Write-Host 'FAILED!'; 		exit 1; 	}; 		Write-Host 'Expanding ...'; 	Expand-Archive go.zip -DestinationPath C:\; 		Write-Host 'Moving ...'; 	Move-Item -Path C:\go -Destination 'C:\Program Files\Go'; 		Write-Host 'Removing ...'; 	Remove-Item go.zip -Force; 		Write-Host 'Verifying install ("go version") ...'; 	go version; 		Write-Host 'Complete.';

 COPY . /neo-go

--- a/cli/server/server.go
+++ b/cli/server/server.go
@ -4,8 +4,10 @@ import (
 	"context"
 	"errors"
 	"fmt"
+	"net/url"
 	"os"
 	"os/signal"
+	"runtime"
 	"syscall"

 	"github.com/nspcc-dev/neo-go/cli/options"
@ -29,6 +31,13 @@ import (
 	"go.uber.org/zap/zapcore"
 )

+var (
+	// _winfileSinkRegistered denotes whether zap has registered
+	// user-supplied factory for all sinks with `winfile`-prefixed scheme.
+	_winfileSinkRegistered bool
+	_winfileSinkCloser     func() error
+)
+
 // NewCommands returns 'node' command.
 func NewCommands() []cli.Command {
 	var cfgFlags = []cli.Flag{
@ -124,7 +133,9 @@ func getConfigFromContext(ctx *cli.Context) (config.Config, error) {
 // handleLoggingParams reads logging parameters.
 // If user selected debug level -- function enables it.
 // If logPath is configured -- function creates dir and file for logging.
-func handleLoggingParams(ctx *cli.Context, cfg config.ApplicationConfiguration) (*zap.Logger, error) {
+// If logPath is configured on Windows -- function returns closer to be
+// able to close sink for opened log output file.
+func handleLoggingParams(ctx *cli.Context, cfg config.ApplicationConfiguration) (*zap.Logger, func() error, error) {
 	level := zapcore.InfoLevel
 	if ctx.Bool("debug") {
 		level = zapcore.DebugLevel
@ -142,13 +153,56 @@ func handleLoggingParams(ctx *cli.Context, cfg config.ApplicationConfiguration)

 	if logPath := cfg.LogPath; logPath != "" {
 		if err := io.MakeDirForFile(logPath, "logger"); err != nil {
-			return nil, err
+			return nil, nil, err
+		}
+
+		if runtime.GOOS == "windows" {
+			if !_winfileSinkRegistered {
+				// See https://github.com/uber-go/zap/issues/621.
+				err := zap.RegisterSink("winfile", func(u *url.URL) (zap.Sink, error) {
+					if u.User != nil {
+						return nil, fmt.Errorf("user and password not allowed with file URLs: got %v", u)
+					}
+					if u.Fragment != "" {
+						return nil, fmt.Errorf("fragments not allowed with file URLs: got %v", u)
+					}
+					if u.RawQuery != "" {
+						return nil, fmt.Errorf("query parameters not allowed with file URLs: got %v", u)
+					}
+					// Error messages are better if we check hostname and port separately.
+					if u.Port() != "" {
+						return nil, fmt.Errorf("ports not allowed with file URLs: got %v", u)
+					}
+					if hn := u.Hostname(); hn != "" && hn != "localhost" {
+						return nil, fmt.Errorf("file URLs must leave host empty or use localhost: got %v", u)
+					}
+					switch u.Path {
+					case "stdout":
+						return os.Stdout, nil
+					case "stderr":
+						return os.Stderr, nil
+					}
+					f, err := os.OpenFile(u.Path[1:], // Remove leading slash left after url.Parse.
+						os.O_WRONLY|os.O_APPEND|os.O_CREATE, 0644)
+					_winfileSinkCloser = func() error {
+						_winfileSinkCloser = nil
+						return f.Close()
+					}
+					return f, err
+				})
+				if err != nil {
+					return nil, nil, fmt.Errorf("failed to register windows-specific sinc: %w", err)
+				}
+				_winfileSinkRegistered = true
+			}
+			logPath = "winfile:///" + logPath
 		}

 		cc.OutputPaths = []string{logPath}
 	}

-	return cc.Build()
+	log, err := cc.Build()
+	return log, _winfileSinkCloser, err
 }

 func initBCWithMetrics(cfg config.Config, log *zap.Logger) (*core.Blockchain, *metrics.Service, *metrics.Service, error) {
@ -172,10 +226,13 @@ func dumpDB(ctx *cli.Context) error {
 	if err != nil {
 		return cli.NewExitError(err, 1)
 	}
-	log, err := handleLoggingParams(ctx, cfg.ApplicationConfiguration)
+	log, logCloser, err := handleLoggingParams(ctx, cfg.ApplicationConfiguration)
 	if err != nil {
 		return cli.NewExitError(err, 1)
 	}
+	if logCloser != nil {
+		defer func() { _ = logCloser() }()
+	}
 	count := uint32(ctx.Uint("count"))
 	start := uint32(ctx.Uint("start"))

@ -219,10 +276,13 @@ func restoreDB(ctx *cli.Context) error {
 	if err != nil {
 		return err
 	}
-	log, err := handleLoggingParams(ctx, cfg.ApplicationConfiguration)
+	log, logCloser, err := handleLoggingParams(ctx, cfg.ApplicationConfiguration)
 	if err != nil {
 		return cli.NewExitError(err, 1)
 	}
+	if logCloser != nil {
+		defer func() { _ = logCloser() }()
+	}
 	count := uint32(ctx.Uint("count"))

 	var inStream = os.Stdin
@ -244,9 +304,11 @@ func restoreDB(ctx *cli.Context) error {
 	if err != nil {
 		return err
 	}
-	defer chain.Close()
-	defer prometheus.ShutDown()
-	defer pprof.ShutDown()
+	defer func() {
+		pprof.ShutDown()
+		prometheus.ShutDown()
+		chain.Close()
+	}()

 	var start uint32
 	if ctx.Bool("incremental") {
@ -395,10 +457,13 @@ func startServer(ctx *cli.Context) error {
 	if err != nil {
 		return cli.NewExitError(err, 1)
 	}
-	log, err := handleLoggingParams(ctx, cfg.ApplicationConfiguration)
+	log, logCloser, err := handleLoggingParams(ctx, cfg.ApplicationConfiguration)
 	if err != nil {
 		return cli.NewExitError(err, 1)
 	}
+	if logCloser != nil {
+		defer func() { _ = logCloser() }()
+	}

 	grace, cancel := context.WithCancel(newGraceContext())
 	defer cancel()
@ -409,6 +474,11 @@ func startServer(ctx *cli.Context) error {
 	if err != nil {
 		return cli.NewExitError(err, 1)
 	}
+	defer func() {
+		pprof.ShutDown()
+		prometheus.ShutDown()
+		chain.Close()
+	}()

 	serv, err := network.NewServer(serverConfig, chain, chain.GetStateSyncModule(), log)
 	if err != nil {
@ -471,9 +541,6 @@ Main:
 			if serverErr := rpcServer.Shutdown(); serverErr != nil {
 				shutdownErr = fmt.Errorf("error on shutdown: %w", serverErr)
 			}
-			prometheus.ShutDown()
-			pprof.ShutDown()
-			chain.Close()
 			break Main
 		}
 	}
--- a/cli/server/server_test.go
+++ b/cli/server/server_test.go
@ -41,7 +41,6 @@ func TestGetConfigFromContext(t *testing.T) {
 }

 func TestHandleLoggingParams(t *testing.T) {
-	// This test is failing on Windows, see https://github.com/nspcc-dev/neo-go/issues/2269
 	d := t.TempDir()
 	testLog := filepath.Join(d, "file.log")

@ -53,8 +52,9 @@ func TestHandleLoggingParams(t *testing.T) {
 		cfg := config.ApplicationConfiguration{
 			LogPath: filepath.Join(logfile, "file.log"),
 		}
-		_, err := handleLoggingParams(ctx, cfg)
+		_, closer, err := handleLoggingParams(ctx, cfg)
 		require.Error(t, err)
+		require.Nil(t, closer)
 	})

 	t.Run("default", func(t *testing.T) {
@ -63,8 +63,13 @@ func TestHandleLoggingParams(t *testing.T) {
 		cfg := config.ApplicationConfiguration{
 			LogPath: testLog,
 		}
-		logger, err := handleLoggingParams(ctx, cfg)
+		logger, closer, err := handleLoggingParams(ctx, cfg)
 		require.NoError(t, err)
+		t.Cleanup(func() {
+			if closer != nil {
+				require.NoError(t, closer())
+			}
+		})
 		require.True(t, logger.Core().Enabled(zap.InfoLevel))
 		require.False(t, logger.Core().Enabled(zap.DebugLevel))
 	})
@ -76,8 +81,13 @@ func TestHandleLoggingParams(t *testing.T) {
 		cfg := config.ApplicationConfiguration{
 			LogPath: testLog,
 		}
-		logger, err := handleLoggingParams(ctx, cfg)
+		logger, closer, err := handleLoggingParams(ctx, cfg)
 		require.NoError(t, err)
+		t.Cleanup(func() {
+			if closer != nil {
+				require.NoError(t, closer())
+			}
+		})
 		require.True(t, logger.Core().Enabled(zap.InfoLevel))
 		require.True(t, logger.Core().Enabled(zap.DebugLevel))
 	})
@ -96,8 +106,13 @@ func TestInitBCWithMetrics(t *testing.T) {
 	ctx := cli.NewContext(cli.NewApp(), set, nil)
 	cfg, err := getConfigFromContext(ctx)
 	require.NoError(t, err)
-	logger, err := handleLoggingParams(ctx, cfg.ApplicationConfiguration)
+	logger, closer, err := handleLoggingParams(ctx, cfg.ApplicationConfiguration)
 	require.NoError(t, err)
+	t.Cleanup(func() {
+		if closer != nil {
+			require.NoError(t, closer())
+		}
+	})

 	t.Run("bad store", func(t *testing.T) {
 		_, _, _, err = initBCWithMetrics(config.Config{}, logger)
--- a/cli/server_test.go
+++ b/cli/server_test.go
@ -1,11 +1,11 @@
 package main

 import (
-	"fmt"
 	"io"
 	"io/ioutil"
 	"os"
 	"path/filepath"
+	"runtime"
 	"strings"
 	"testing"
 	"time"
@ -96,29 +96,33 @@ func TestServerStart(t *testing.T) {
 			e.RunWithError(t, baseCmd...)
 		})
 	})
-	t.Run("good", func(t *testing.T) {
-		saveCfg(t, func(cfg *config.Config) {})
+	// We can't properly shutdown server on windows and release the resources.
+	// Also, windows doesn't support SIGHUP and SIGINT.
+	if runtime.GOOS != "windows" {
+		t.Run("good", func(t *testing.T) {
+			saveCfg(t, func(cfg *config.Config) {})

-		go func() {
-			e.Run(t, baseCmd...)
-		}()
+			go func() {
+				e.Run(t, baseCmd...)
+			}()

-		var line string
-		require.Eventually(t, func() bool {
-			line, err = e.Out.ReadString('\n')
-			if err != nil && err != io.EOF {
-				t.Fatalf(fmt.Sprintf("unexpected error while reading CLI output: %s", err))
+			var line string
+			require.Eventually(t, func() bool {
+				line, err = e.Out.ReadString('\n')
+				if err != nil && err != io.EOF {
+					t.Fatalf("unexpected error while reading CLI output: %s", err)
+				}
+				return err == nil
+			}, 2*time.Second, 100*time.Millisecond)
+			lines := strings.Split(server.Logo(), "\n")
+			for _, expected := range lines {
+				// It should be regexp, so escape all backslashes.
+				expected = strings.ReplaceAll(expected, `\`, `\\`)
+				e.checkLine(t, line, expected)
+				line = e.getNextLine(t)
 			}
-			return err == nil
-		}, 2*time.Second, 100*time.Millisecond)
-		lines := strings.Split(server.Logo(), "\n")
-		for _, expected := range lines {
-			// It should be regexp, so escape all backslashes.
-			expected = strings.ReplaceAll(expected, `\`, `\\`)
-			e.checkLine(t, line, expected)
-			line = e.getNextLine(t)
-		}
-		e.checkNextLine(t, "")
-		e.checkEOF(t)
-	})
+			e.checkNextLine(t, "")
+			e.checkEOF(t)
+		})
+	}
 }
--- a/pkg/compiler/analysis.go
+++ b/pkg/compiler/analysis.go
@ -235,9 +235,10 @@ func (c *codegen) fillDocumentInfo() {
 	fset := c.buildInfo.config.Fset
 	fset.Iterate(func(f *token.File) bool {
 		filePath := f.Position(f.Pos(0)).Filename
-		filePath, err := filepath.Rel(c.buildInfo.config.Dir, filePath)
-		if err != nil {
-			panic(err)
+		rel, err := filepath.Rel(c.buildInfo.config.Dir, filePath)
+		// It's OK if we can't construct relative path, e.g. for interop dependencies.
+		if err == nil {
+			filePath = rel
 		}
 		c.docIndex[filePath] = len(c.documents)
 		c.documents = append(c.documents, filePath)
--- a/pkg/network/server_test.go
+++ b/pkg/network/server_test.go
@ -404,7 +404,7 @@ func TestBlock(t *testing.T) {
 	b := block.New(false)
 	b.Index = 12345
 	s.testHandleMessage(t, nil, CMDBlock, b)
-	require.Eventually(t, func() bool { return s.chain.BlockHeight() == 12345 }, time.Second, time.Millisecond*500)
+	require.Eventually(t, func() bool { return s.chain.BlockHeight() == 12345 }, 2*time.Second, time.Millisecond*500)
 }

 func TestConsensus(t *testing.T) {
@ -497,8 +497,8 @@ func (s *Server) testHandleGetData(t *testing.T, invType payload.InventoryType,

 	s.testHandleMessage(t, p, CMDGetData, payload.NewInventory(invType, hs))

-	require.Eventually(t, func() bool { return recvResponse.Load() }, time.Second, time.Millisecond)
-	require.Eventually(t, func() bool { return recvNotFound.Load() }, time.Second, time.Millisecond)
+	require.Eventually(t, func() bool { return recvResponse.Load() }, 2*time.Second, time.Millisecond)
+	require.Eventually(t, func() bool { return recvNotFound.Load() }, 2*time.Second, time.Millisecond)
 }

 func TestGetData(t *testing.T) {
--- a/pkg/vm/cli/cli_test.go
+++ b/pkg/vm/cli/cli_test.go
@ -84,6 +84,10 @@ func newTestVMCLIWithLogo(t *testing.T, printLogo bool) *executor {
 }

 func (e *executor) runProg(t *testing.T, commands ...string) {
+	e.runProgWithTimeout(t, 4*time.Second, commands...)
+}
+
+func (e *executor) runProgWithTimeout(t *testing.T, timeout time.Duration, commands ...string) {
 	cmd := strings.Join(commands, "\n") + "\n"
 	e.in.WriteString(cmd + "\n")
 	go func() {
@ -92,7 +96,7 @@ func (e *executor) runProg(t *testing.T, commands ...string) {
 	}()
 	select {
 	case <-e.ch:
-	case <-time.After(4 * time.Second):
+	case <-time.After(timeout):
 		require.Fail(t, "command took too long time")
 	}
 }
@ -213,7 +217,7 @@ go 1.16`)
 		require.NoError(t, ioutil.WriteFile(filepath.Join(tmpDir, "go.mod"), goMod, os.ModePerm))

 		e := newTestVMCLI(t)
-		e.runProg(t,
+		e.runProgWithTimeout(t, 10*time.Second,
 			"loadgo",
 			"loadgo "+filenameErr,
 			"loadgo "+filename,
@ -318,7 +322,7 @@ func TestRunWithDifferentArguments(t *testing.T) {

 	filename = "'" + filename + "'"
 	e := newTestVMCLI(t)
-	e.runProg(t,
+	e.runProgWithTimeout(t, 30*time.Second,
 		"loadgo "+filename, "run notexists",
 		"loadgo "+filename, "run negate false",
 		"loadgo "+filename, "run negate true",