diff options
133 files changed, 1607 insertions, 1246 deletions
diff --git a/.buildkite/Makefile b/.buildkite/Makefile index b039b73ba53..b5f730d0133 100644 --- a/.buildkite/Makefile +++ b/.buildkite/Makefile @@ -29,7 +29,7 @@ endif .DEFAULT_GOAL := pr -main: build-rpms cpp-test quick-start-guide publish-container publish-artifacts +main: build-rpms cpp-test quick-start-guide publish-container publish-artifacts upload-test-results pr: build-rpms cpp-test basic-search-test check: @@ -74,6 +74,9 @@ publish-container: build-container publish-artifacts: java build-rpms @$(TOP)/execute.sh $@ +upload-test-results: java cpp-test + @$(TOP)/execute.sh $@ + .PHONY: \ main \ pr \ @@ -90,4 +93,5 @@ publish-artifacts: java build-rpms quick-start-guide \ publish-container \ publish-artifacts \ + upload-test-results \ check diff --git a/.buildkite/upload-test-results.sh b/.buildkite/upload-test-results.sh new file mode 100755 index 00000000000..e95a9448adf --- /dev/null +++ b/.buildkite/upload-test-results.sh @@ -0,0 +1,55 @@ +#!/bin/bash + +set -euo pipefail + +if [[ $BUILDKITE != true ]]; then + echo "Skipping artifact publishing when not executed by Buildkite." + exit 0 +fi + +if [[ $(arch) == x86_64 ]]; then + JAVA_TEST_TOKEN=$UNIT_TEST_JAVA_AMD64_TOKEN + CPP_TEST_TOKEN=$UNIT_TEST_CPP_AMD64_TOKEN +else + JAVA_TEST_TOKEN=$UNIT_TEST_JAVA_ARM64_TOKEN + CPP_TEST_TOKEN=$UNIT_TEST_CPP_ARM64_TOKEN +fi + +if [[ -z $JAVA_TEST_TOKEN ]]; then + echo "Missing JAVA_TEST_TOKEN. Exiting." + exit 1 +fi +if [[ -z $CPP_TEST_TOKEN ]]; then + echo "Missing CPP_TEST_TOKEN. Exiting." + exit 1 +fi + +upload_result() { + curl \ + -X POST \ + -H "Authorization: Token token=\"$BUILDKITE_ANALYTICS_TOKEN\"" \ + -F "data=@$1" \ + -F "format=junit" \ + -F "run_env[CI]=buildkite" \ + -F "run_env[key]=$BUILDKITE_BUILD_ID" \ + -F "run_env[url]=$BUILDKITE_BUILD_URL" \ + -F "run_env[branch]=$BUILDKITE_BRANCH" \ + -F "run_env[commit_sha]=$BUILDKITE_COMMIT" \ + -F "run_env[number]=$BUILDKITE_BUILD_NUMBER" \ + -F "run_env[job_id]=$BUILDKITE_JOB_ID" \ + -F "run_env[message]=$BUILDKITE_MESSAGE" \ + "https://analytics-api.buildkite.com/v1/uploads" +} + +export -f upload_result + +# Upload all surefire TEST-*.xml reports +cd "$WORKDIR" +export BUILDKITE_ANALYTICS_TOKEN=$JAVA_TEST_TOKEN +# shellcheck disable=2038 +find . -name "TEST-*.xml" -type f | xargs -n 1 -P 50 -I '{}' bash -c "upload_result {}" + +# Upload the cpp test report +export BUILDKITE_ANALYTICS_TOKEN=$CPP_TEST_TOKEN +upload_result "$LOG_DIR/vespa-cpptest-results.xml" + diff --git a/client/go/Makefile b/client/go/Makefile index b2ffdc0feb6..fee92547e73 100644 --- a/client/go/Makefile +++ b/client/go/Makefile @@ -17,6 +17,7 @@ GO_FLAGS := -ldflags "-X github.com/vespa-engine/vespa/client/go/internal/build. PROJECT_ROOT := $(shell realpath $(CURDIR)/../..) GO_TMPDIR := $(PROJECT_ROOT)/build/go DIST_TARGETS := dist-mac dist-mac-arm64 dist-linux dist-linux-arm64 dist-win32 dist-win64 +GOTOOLCHAIN := $(shell go env GOTOOLCHAIN) all: test checkfmt install @@ -111,7 +112,9 @@ install-all: all manpages # Development targets # -ci: +setenv: +# Set GOTOOLCHAIN if its default value has been changed + @test "$(GOTOOLCHAIN)" = auto || go env -w GOTOOLCHAIN="auto" ifdef CI # Ensure that CI systems use a proxy for downloading dependencies go env -w GOPROXY="https://proxy.golang.org,direct" @@ -121,7 +124,7 @@ endif install-brew: brew install vespa-cli -install: ci +install: setenv env GOBIN=$(BIN) go install $(GO_FLAGS) ./... manpages: install @@ -133,7 +136,7 @@ clean: rm -f $(BIN)/vespa $(BIN)/vespa-wrapper $(SHARE)/man/man1/vespa.1 $(SHARE)/man/man1/vespa-*.1 rmdir -p $(BIN) $(SHARE)/man/man1 > /dev/null 2>&1 || true -test: ci +test: setenv # Why custom GOTMPDIR? go builds executables for unit tests and by default these # end up in TMPDIR/GOTMPDIR. In some environments /tmp is mounted noexec so # running test executables will fail diff --git a/client/go/go.mod b/client/go/go.mod index 27ed546cd11..4b6b0ceef49 100644 --- a/client/go/go.mod +++ b/client/go/go.mod @@ -1,13 +1,12 @@ module github.com/vespa-engine/vespa/client/go -go 1.21 +go 1.22.4 require ( github.com/alessio/shellescape v1.4.2 github.com/briandowns/spinner v1.23.1 github.com/fatih/color v1.17.0 - // This is the most recent version compatible with Go 1.21. Upgrade when we upgrade our Go version - github.com/go-json-experiment/json v0.0.0-20240412061110-8868a69194fa + github.com/go-json-experiment/json v0.0.0-20240524174822-2d9f40f7385b github.com/klauspost/compress v1.17.9 github.com/mattn/go-colorable v0.1.13 github.com/mattn/go-isatty v0.0.20 @@ -30,7 +29,6 @@ require ( github.com/kr/pretty v0.1.0 // indirect github.com/pmezard/go-difflib v1.0.0 // indirect github.com/russross/blackfriday/v2 v2.1.0 // indirect - golang.org/x/exp v0.0.0-20231110203233-9a3e6036ecaa // indirect golang.org/x/term v0.21.0 // indirect golang.org/x/text v0.16.0 // indirect gopkg.in/check.v1 v1.0.0-20190902080502-41f04d3bba15 // indirect diff --git a/client/go/go.sum b/client/go/go.sum index 1c2f791079b..d8b6a4cacf9 100644 --- a/client/go/go.sum +++ b/client/go/go.sum @@ -1,41 +1,21 @@ github.com/alessio/shellescape v1.4.2 h1:MHPfaU+ddJ0/bYWpgIeUnQUqKrlJ1S7BfEYPM4uEoM0= github.com/alessio/shellescape v1.4.2/go.mod h1:PZAiSCk0LJaZkiCSkPv8qIobYglO3FPpyFjDCtHLS30= -github.com/briandowns/spinner v1.23.0 h1:alDF2guRWqa/FOZZYWjlMIx2L6H0wyewPxo/CH4Pt2A= -github.com/briandowns/spinner v1.23.0/go.mod h1:rPG4gmXeN3wQV/TsAY4w8lPdIM6RX3yqeBQJSrbXjuE= github.com/briandowns/spinner v1.23.1 h1:t5fDPmScwUjozhDj4FA46p5acZWIPXYE30qW2Ptu650= github.com/briandowns/spinner v1.23.1/go.mod h1:LaZeM4wm2Ywy6vO571mvhQNRcWfRUnXOs0RcKV0wYKM= -github.com/cpuguy83/go-md2man/v2 v2.0.3 h1:qMCsGGgs+MAzDFyp9LpAe1Lqy/fY/qCovCm0qnXZOBM= -github.com/cpuguy83/go-md2man/v2 v2.0.3/go.mod h1:tgQtvFlXSQOSOSIRvRPT7W67SCa46tRHOmNcaadrF8o= github.com/cpuguy83/go-md2man/v2 v2.0.4 h1:wfIWP927BUkWJb2NmU/kNDYIBTh/ziUX91+lVfRxZq4= github.com/cpuguy83/go-md2man/v2 v2.0.4/go.mod h1:tgQtvFlXSQOSOSIRvRPT7W67SCa46tRHOmNcaadrF8o= github.com/danieljoos/wincred v1.2.0 h1:ozqKHaLK0W/ii4KVbbvluM91W2H3Sh0BncbUNPS7jLE= github.com/danieljoos/wincred v1.2.0/go.mod h1:FzQLLMKBFdvu+osBrnFODiv32YGwCfx0SkRa/eYHgec= github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c= github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= -github.com/fatih/color v1.16.0 h1:zmkK9Ngbjj+K0yRhTVONQh1p/HknKYSlNT+vZCzyokM= -github.com/fatih/color v1.16.0/go.mod h1:fL2Sau1YI5c0pdGEVCbKQbLXB6edEj1ZgiY4NijnWvE= github.com/fatih/color v1.17.0 h1:GlRw1BRJxkpqUCBKzKOw098ed57fEsKeNjpTe3cSjK4= github.com/fatih/color v1.17.0/go.mod h1:YZ7TlrGPkiz6ku9fK3TLD/pl3CpsiFyu8N92HLgmosI= -github.com/go-json-experiment/json v0.0.0-20230324203220-04923b7a9528 h1:hmpF6G+rHcypt8J6jhBH/rDUx+04Th/L61Y8uCKFb7Q= -github.com/go-json-experiment/json v0.0.0-20230324203220-04923b7a9528/go.mod h1:AHV+bpNGVGD0DCHMBhhTYtT7yeBYD9Yk92XAjB7vOgo= -github.com/go-json-experiment/json v0.0.0-20240412061110-8868a69194fa h1:JUl7LfZewNlppx7w/82EwZ7d/N2nTXncAOeE55tI3Pk= -github.com/go-json-experiment/json v0.0.0-20240412061110-8868a69194fa/go.mod h1:6daplAwHHGbUGib4990V3Il26O0OC4aRyvewaaAihaA= +github.com/go-json-experiment/json v0.0.0-20240524174822-2d9f40f7385b h1:IM96IiRXFcd7l+mU8Sys9pcggoBLbH/dEgzOESrS8F8= +github.com/go-json-experiment/json v0.0.0-20240524174822-2d9f40f7385b/go.mod h1:uDEMZSTQMj7V6Lxdrx4ZwchmHEGdICbjuY+GQd7j9LM= github.com/godbus/dbus/v5 v5.1.0 h1:4KLkAxT3aOY8Li4FRJe/KvhoNFFxo0m6fNuFUO8QJUk= github.com/godbus/dbus/v5 v5.1.0/go.mod h1:xhWf0FNVPg57R7Z0UbKHbJfkEywrmjJnf7w5xrFpKfA= github.com/inconshreveable/mousetrap v1.1.0 h1:wN+x4NVGpMsO7ErUn/mUI3vEoE6Jt13X2s0bqwp9tc8= github.com/inconshreveable/mousetrap v1.1.0/go.mod h1:vpF70FUmC8bwa3OWnCshd2FqLfsEA9PFc4w1p2J65bw= -github.com/klauspost/compress v1.17.3 h1:qkRjuerhUU1EmXLYGkSH6EZL+vPSxIrYjLNAK4slzwA= -github.com/klauspost/compress v1.17.3/go.mod h1:/dCuZOvVtNoHsyb+cuJD3itjs3NbnF6KH9zAO4BDxPM= -github.com/klauspost/compress v1.17.4 h1:Ej5ixsIri7BrIjBkRZLTo6ghwrEtHFk7ijlczPW4fZ4= -github.com/klauspost/compress v1.17.4/go.mod h1:/dCuZOvVtNoHsyb+cuJD3itjs3NbnF6KH9zAO4BDxPM= -github.com/klauspost/compress v1.17.5 h1:d4vBd+7CHydUqpFBgUEKkSdtSugf9YFmSkvUYPquI5E= -github.com/klauspost/compress v1.17.5/go.mod h1:/dCuZOvVtNoHsyb+cuJD3itjs3NbnF6KH9zAO4BDxPM= -github.com/klauspost/compress v1.17.6 h1:60eq2E/jlfwQXtvZEeBUYADs+BwKBWURIY+Gj2eRGjI= -github.com/klauspost/compress v1.17.6/go.mod h1:/dCuZOvVtNoHsyb+cuJD3itjs3NbnF6KH9zAO4BDxPM= -github.com/klauspost/compress v1.17.7 h1:ehO88t2UGzQK66LMdE8tibEd1ErmzZjNEqWkjLAKQQg= -github.com/klauspost/compress v1.17.7/go.mod h1:Di0epgTjJY877eYKx5yC51cX2A2Vl2ibi7bDH9ttBbw= -github.com/klauspost/compress v1.17.8 h1:YcnTYrq7MikUT7k0Yb5eceMmALQPYBW/Xltxn0NAMnU= -github.com/klauspost/compress v1.17.8/go.mod h1:Di0epgTjJY877eYKx5yC51cX2A2Vl2ibi7bDH9ttBbw= github.com/klauspost/compress v1.17.9 h1:6KIumPrER1LHsvBVuDa0r5xaG0Es51mhhB9BQB2qeMA= github.com/klauspost/compress v1.17.9/go.mod h1:Di0epgTjJY877eYKx5yC51cX2A2Vl2ibi7bDH9ttBbw= github.com/kr/pretty v0.1.0 h1:L/CwN0zerZDmRFUapSPitk6f+Q3+0za1rQkzVuMiMFI= @@ -48,91 +28,31 @@ github.com/mattn/go-colorable v0.1.13/go.mod h1:7S9/ev0klgBDR4GtXTXX8a3vIGJpMovk github.com/mattn/go-isatty v0.0.16/go.mod h1:kYGgaQfpe5nmfYZH+SKPsOc2e4SrIfOl2e/yFXSvRLM= github.com/mattn/go-isatty v0.0.20 h1:xfD0iDuEKnDkl03q4limB+vH+GxLEtL/jb4xVJSWWEY= github.com/mattn/go-isatty v0.0.20/go.mod h1:W+V8PltTTMOvKvAeJH7IuucS94S2C6jfK/D7dTCTo3Y= -github.com/pkg/browser v0.0.0-20210911075715-681adbf594b8 h1:KoWmjvw+nsYOo29YJK9vDA65RGE3NrOnUtO7a+RF9HU= -github.com/pkg/browser v0.0.0-20210911075715-681adbf594b8/go.mod h1:HKlIX3XHQyzLZPlr7++PzdhaXEj94dEiJgZDTsxEqUI= github.com/pkg/browser v0.0.0-20240102092130-5ac0b6a4141c h1:+mdjkGKdHQG3305AYmdv1U2eRNDiU2ErMBj1gwrq8eQ= github.com/pkg/browser v0.0.0-20240102092130-5ac0b6a4141c/go.mod h1:7rwL4CYBLnjLxUqIJNnCWiEdr3bn6IUYi15bNlnbCCU= github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM= github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4= github.com/russross/blackfriday/v2 v2.1.0 h1:JIOH55/0cWyOuilr9/qlrm0BSXldqnqwMsf35Ld67mk= github.com/russross/blackfriday/v2 v2.1.0/go.mod h1:+Rmxgy9KzJVeS9/2gXHxylqXiyQDYRxCVz55jmeOWTM= -github.com/spf13/cobra v1.8.0 h1:7aJaZx1B85qltLMc546zn58BxxfZdR/W22ej9CFoEf0= -github.com/spf13/cobra v1.8.0/go.mod h1:WXLWApfZ71AjXPya3WOlMsY9yMs7YeiHhFVlvLyhcho= github.com/spf13/cobra v1.8.1 h1:e5/vxKd/rZsfSJMUX1agtjeTDf+qv1/JdBF8gg5k9ZM= github.com/spf13/cobra v1.8.1/go.mod h1:wHxEcudfqmLYa8iTfL+OuZPbBZkmvliBWKIezN3kD9Y= github.com/spf13/pflag v1.0.5 h1:iy+VFUOCP1a+8yFto/drg2CJ5u0yRoB7fZw3DKv/JXA= github.com/spf13/pflag v1.0.5/go.mod h1:McXfInJRrz4CZXVZOBLb0bTZqETkiAhM9Iw0y3An2Bg= -github.com/stretchr/objx v0.5.0 h1:1zr/of2m5FGMsad5YfcqgdqdWrIhu+EBEJRhR1U7z/c= -github.com/stretchr/testify v1.8.4 h1:CcVxjf3Q8PM0mHUKJCdn+eZZtm5yQwehR5yeSVQQcUk= -github.com/stretchr/testify v1.8.4/go.mod h1:sz/lmYIOXD/1dqDmKjjqLyZ2RngseejIcXlSw2iwfAo= +github.com/stretchr/objx v0.5.2 h1:xuMeJ0Sdp5ZMRXx/aWO6RZxdr3beISkG5/G/aIRr3pY= +github.com/stretchr/objx v0.5.2/go.mod h1:FRsXN1f5AsAjCGJKqEizvkpNtU+EGNCLh3NxZ/8L+MA= github.com/stretchr/testify v1.9.0 h1:HtqpIVDClZ4nwg75+f6Lvsy/wHu+3BoSGCbBAcpTsTg= github.com/stretchr/testify v1.9.0/go.mod h1:r2ic/lqez/lEtzL7wO/rwa5dbSLXVDPFyf8C91i36aY= -github.com/zalando/go-keyring v0.2.3 h1:v9CUu9phlABObO4LPWycf+zwMG7nlbb3t/B5wa97yms= -github.com/zalando/go-keyring v0.2.3/go.mod h1:HL4k+OXQfJUWaMnqyuSOc0drfGPX2b51Du6K+MRgZMk= -github.com/zalando/go-keyring v0.2.4 h1:wi2xxTqdiwMKbM6TWwi+uJCG/Tum2UV0jqaQhCa9/68= -github.com/zalando/go-keyring v0.2.4/go.mod h1:HL4k+OXQfJUWaMnqyuSOc0drfGPX2b51Du6K+MRgZMk= github.com/zalando/go-keyring v0.2.5 h1:Bc2HHpjALryKD62ppdEzaFG6VxL6Bc+5v0LYpN8Lba8= github.com/zalando/go-keyring v0.2.5/go.mod h1:HL4k+OXQfJUWaMnqyuSOc0drfGPX2b51Du6K+MRgZMk= -golang.org/x/exp v0.0.0-20231110203233-9a3e6036ecaa h1:FRnLl4eNAQl8hwxVVC17teOw8kdjVDVAiFMtgUdTSRQ= -golang.org/x/exp v0.0.0-20231110203233-9a3e6036ecaa/go.mod h1:zk2irFbV9DP96SEBUUAy67IdHUaZuSnrz1n472HUCLE= -golang.org/x/net v0.18.0 h1:mIYleuAkSbHh0tCv7RvjL3F6ZVbLjq4+R7zbOn3Kokg= -golang.org/x/net v0.18.0/go.mod h1:/czyP5RqHAH4odGYxBJ1qz0+CE5WZ+2j1YgoEo8F2jQ= -golang.org/x/net v0.19.0 h1:zTwKpTd2XuCqf8huc7Fo2iSy+4RHPd10s4KzeTnVr1c= -golang.org/x/net v0.19.0/go.mod h1:CfAk/cbD4CthTvqiEl8NpboMuiuOYsAr/7NOjZJtv1U= -golang.org/x/net v0.20.0 h1:aCL9BSgETF1k+blQaYUBx9hJ9LOGP3gAVemcZlf1Kpo= -golang.org/x/net v0.20.0/go.mod h1:z8BVo6PvndSri0LbOE3hAn0apkU+1YvI6E70E9jsnvY= -golang.org/x/net v0.21.0 h1:AQyQV4dYCvJ7vGmJyKki9+PBdyvhkSd8EIx/qb0AYv4= -golang.org/x/net v0.21.0/go.mod h1:bIjVDfnllIU7BJ2DNgfnXvpSvtn8VRwhlsaeUTyUS44= -golang.org/x/net v0.22.0 h1:9sGLhx7iRIHEiX0oAJ3MRZMUCElJgy7Br1nO+AMN3Tc= -golang.org/x/net v0.22.0/go.mod h1:JKghWKKOSdJwpW2GEx0Ja7fmaKnMsbu+MWVZTokSYmg= -golang.org/x/net v0.23.0 h1:7EYJ93RZ9vYSZAIb2x3lnuvqO5zneoD6IvWjuhfxjTs= -golang.org/x/net v0.23.0/go.mod h1:JKghWKKOSdJwpW2GEx0Ja7fmaKnMsbu+MWVZTokSYmg= -golang.org/x/net v0.24.0 h1:1PcaxkF854Fu3+lvBIx5SYn9wRlBzzcnHZSiaFFAb0w= -golang.org/x/net v0.24.0/go.mod h1:2Q7sJY5mzlzWjKtYUEXSlBWCdyaioyXzRB2RtU8KVE8= -golang.org/x/net v0.25.0 h1:d/OCCoBEUq33pjydKrGQhw7IlUPI2Oylr+8qLx49kac= -golang.org/x/net v0.25.0/go.mod h1:JkAGAh7GEvH74S6FOH42FLoXpXbE/aqXSrIQjXgsiwM= golang.org/x/net v0.26.0 h1:soB7SVo0PWrY4vPW/+ay0jKDNScG2X9wFeYlXIvJsOQ= golang.org/x/net v0.26.0/go.mod h1:5YKkiSynbBIh3p6iOc/vibscux0x38BZDkn8sCUPxHE= -golang.org/x/sys v0.0.0-20210616045830-e2b7044e8c71/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.0.0-20220811171246-fbc7d0a398ab/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.1.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.6.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= -golang.org/x/sys v0.14.0 h1:Vz7Qs629MkJkGyHxUlRHizWJRG2j8fbQKjELVSNhy7Q= -golang.org/x/sys v0.14.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA= -golang.org/x/sys v0.15.0 h1:h48lPFYpsTvQJZF4EKyI4aLHaev3CxivZmv7yZig9pc= -golang.org/x/sys v0.15.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA= -golang.org/x/sys v0.16.0 h1:xWw16ngr6ZMtmxDyKyIgsE93KNKz5HKmMa3b8ALHidU= -golang.org/x/sys v0.16.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA= -golang.org/x/sys v0.17.0 h1:25cE3gD+tdBA7lp7QfhuV+rJiE9YXTcS3VG1SqssI/Y= -golang.org/x/sys v0.17.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA= -golang.org/x/sys v0.18.0 h1:DBdB3niSjOA/O0blCZBqDefyWNYveAYMNF1Wum0DYQ4= -golang.org/x/sys v0.18.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA= -golang.org/x/sys v0.19.0 h1:q5f1RH2jigJ1MoAWp2KTp3gm5zAGFUTarQZ5U386+4o= -golang.org/x/sys v0.19.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA= -golang.org/x/sys v0.20.0 h1:Od9JTbYCk261bKm4M/mw7AklTlFYIa0bIp9BgSm1S8Y= -golang.org/x/sys v0.20.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA= golang.org/x/sys v0.21.0 h1:rF+pYz3DAGSQAxAu1CbC7catZg4ebC4UIeIhKxBZvws= golang.org/x/sys v0.21.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA= -golang.org/x/term v0.14.0 h1:LGK9IlZ8T9jvdy6cTdfKUCltatMFOehAQo9SRC46UQ8= -golang.org/x/term v0.14.0/go.mod h1:TySc+nGkYR6qt8km8wUhuFRTVSMIX3XPR58y2lC8vww= -golang.org/x/term v0.15.0 h1:y/Oo/a/q3IXu26lQgl04j/gjuBDOBlx7X6Om1j2CPW4= -golang.org/x/term v0.15.0/go.mod h1:BDl952bC7+uMoWR75FIrCDx79TPU9oHkTZ9yRbYOrX0= -golang.org/x/term v0.16.0 h1:m+B6fahuftsE9qjo0VWp2FW0mB3MTJvR0BaMQrq0pmE= -golang.org/x/term v0.16.0/go.mod h1:yn7UURbUtPyrVJPGPq404EukNFxcm/foM+bV/bfcDsY= -golang.org/x/term v0.17.0 h1:mkTF7LCd6WGJNL3K1Ad7kwxNfYAW6a8a8QqtMblp/4U= -golang.org/x/term v0.17.0/go.mod h1:lLRBjIVuehSbZlaOtGMbcMncT+aqLLLmKrsjNrUguwk= -golang.org/x/term v0.18.0 h1:FcHjZXDMxI8mM3nwhX9HlKop4C0YQvCVCdwYl2wOtE8= -golang.org/x/term v0.18.0/go.mod h1:ILwASektA3OnRv7amZ1xhE/KTR+u50pbXfZ03+6Nx58= -golang.org/x/term v0.19.0 h1:+ThwsDv+tYfnJFhF4L8jITxu1tdTWRTZpdsWgEgjL6Q= -golang.org/x/term v0.19.0/go.mod h1:2CuTdWZ7KHSQwUzKva0cbMg6q2DMI3Mmxp+gKJbskEk= -golang.org/x/term v0.20.0 h1:VnkxpohqXaOBYJtBmEppKUG6mXpi+4O6purfc2+sMhw= -golang.org/x/term v0.20.0/go.mod h1:8UkIAJTvZgivsXaD6/pH6U9ecQzZ45awqEOzuCvwpFY= golang.org/x/term v0.21.0 h1:WVXCp+/EBEHOj53Rvu+7KiT/iElMrO8ACK16SMZ3jaA= golang.org/x/term v0.21.0/go.mod h1:ooXLefLobQVslOqselCNF4SxFAaoS6KujMbsGzSDmX0= -golang.org/x/text v0.14.0 h1:ScX5w1eTa3QqT8oi6+ziP7dTV1S2+ALU0bI+0zXKWiQ= -golang.org/x/text v0.14.0/go.mod h1:18ZOQIKpY8NJVqYksKHtTdi31H5itFRjB5/qKTNYzSU= -golang.org/x/text v0.15.0 h1:h1V/4gjBv8v9cjcR6+AR5+/cIYK5N/WAgiv4xlsEtAk= -golang.org/x/text v0.15.0/go.mod h1:18ZOQIKpY8NJVqYksKHtTdi31H5itFRjB5/qKTNYzSU= golang.org/x/text v0.16.0 h1:a94ExnEXNtEwYLGJSIUxnWoxoRz/ZcCsV63ROupILh4= golang.org/x/text v0.16.0/go.mod h1:GhwF1Be+LQoKShO3cGOHzqOgRrGaYc9AvblQOmPVHnI= gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= diff --git a/client/go/internal/admin/jvm/mem_options_test.go b/client/go/internal/admin/jvm/mem_options_test.go index 3db10153086..60cffc824e9 100644 --- a/client/go/internal/admin/jvm/mem_options_test.go +++ b/client/go/internal/admin/jvm/mem_options_test.go @@ -9,7 +9,7 @@ import ( func TestAdjustment(t *testing.T) { lastAdj := 64 - for i := 0; i < 4096; i++ { + for i := range 4096 { adj := adjustAvailableMemory(MegaBytesOfMemory(i)).ToMB() assert.True(t, int(adj) >= lastAdj) lastAdj = int(adj) diff --git a/client/go/internal/admin/vespa-wrapper/logfmt/tail_unix.go b/client/go/internal/admin/vespa-wrapper/logfmt/tail_unix.go index ec2c53487be..180f7c84859 100644 --- a/client/go/internal/admin/vespa-wrapper/logfmt/tail_unix.go +++ b/client/go/internal/admin/vespa-wrapper/logfmt/tail_unix.go @@ -85,7 +85,7 @@ func (t *unixTail) openTail() { if err != nil { return } - for i := 0; i < n; i++ { + for i := range n { if t.lineBuf[i] == '\n' { sz, err = file.Seek(sz+int64(i+1), os.SEEK_SET) if err == nil { diff --git a/client/go/internal/cli/cmd/feed.go b/client/go/internal/cli/cmd/feed.go index 6c5df8b3e84..d6bc59f5b4f 100644 --- a/client/go/internal/cli/cmd/feed.go +++ b/client/go/internal/cli/cmd/feed.go @@ -136,7 +136,7 @@ func createServices(n int, timeout time.Duration, cli *CLI, waiter *Waiter) ([]h } services := make([]httputil.Client, 0, n) baseURL := "" - for i := 0; i < n; i++ { + for range n { service, err := waiter.Service(target, cli.config.cluster()) if err != nil { return nil, "", err diff --git a/client/go/internal/cli/cmd/feed_test.go b/client/go/internal/cli/cmd/feed_test.go index 200a0be7c5d..fc25f8e872c 100644 --- a/client/go/internal/cli/cmd/feed_test.go +++ b/client/go/internal/cli/cmd/feed_test.go @@ -82,13 +82,13 @@ func TestFeed(t *testing.T) { require.Nil(t, cli.Run("feed", "-")) assert.Equal(t, want, stdout.String()) - for i := 0; i < 10; i++ { + for range 10 { httpClient.NextResponseString(503, `{"message":"it's broken yo"}`) } require.Nil(t, cli.Run("feed", jsonFile1)) assert.Equal(t, "feed: got status 503 ({\"message\":\"it's broken yo\"}) for put id:ns:type::doc1: giving up after 10 attempts\n", stderr.String()) stderr.Reset() - for i := 0; i < 10; i++ { + for range 10 { httpClient.NextResponseError(fmt.Errorf("something else is broken")) } require.Nil(t, cli.Run("feed", jsonFile1)) diff --git a/client/go/internal/cli/cmd/query.go b/client/go/internal/cli/cmd/query.go index 4d5941943ca..5fa225777f0 100644 --- a/client/go/internal/cli/cmd/query.go +++ b/client/go/internal/cli/cmd/query.go @@ -84,7 +84,7 @@ func query(cli *CLI, arguments []string, timeoutSecs int, curl bool, format stri } url, _ := url.Parse(service.BaseURL + "/search/") urlQuery := url.Query() - for i := 0; i < len(arguments); i++ { + for i := range len(arguments) { key, value := splitArg(arguments[i]) urlQuery.Set(key, value) } diff --git a/client/go/internal/cli/cmd/status_test.go b/client/go/internal/cli/cmd/status_test.go index bf9b4f3493e..6db27fd2778 100644 --- a/client/go/internal/cli/cmd/status_test.go +++ b/client/go/internal/cli/cmd/status_test.go @@ -168,7 +168,7 @@ func TestStatusCloudDeployment(t *testing.T) { } func isLocalTarget(args []string) bool { - for i := 0; i < len(args)-1; i++ { + for i := range len(args) - 1 { if args[i] == "-t" { return args[i+1] == "local" } @@ -197,7 +197,7 @@ func assertStatus(expectedTarget string, args []string, t *testing.T) { t.Helper() client := &mock.HTTPClient{} clusterName := "" - for i := 0; i < 3; i++ { + for range 3 { if isLocalTarget(args) { clusterName = "foo" mockServiceStatus(client, clusterName) diff --git a/client/go/internal/cli/cmd/test_test.go b/client/go/internal/cli/cmd/test_test.go index 728e8c29691..3479e057e45 100644 --- a/client/go/internal/cli/cmd/test_test.go +++ b/client/go/internal/cli/cmd/test_test.go @@ -26,11 +26,11 @@ func TestSuite(t *testing.T) { mockServiceStatus(client, "container") client.NextStatus(200) client.NextStatus(200) - for i := 0; i < 2; i++ { + for range 2 { client.NextResponseString(200, string(searchResponse)) } mockServiceStatus(client, "container") // Some tests do not specify cluster, which is fine since we only have one, but this causes a cache miss - for i := 0; i < 9; i++ { + for range 9 { client.NextResponseString(200, string(searchResponse)) } expectedBytes, _ := os.ReadFile("testdata/tests/expected-suite.out") @@ -45,7 +45,7 @@ func TestSuite(t *testing.T) { requests = append(requests, discoveryRequest) requests = append(requests, createSearchRequest(baseUrl+"/search/")) requests = append(requests, createSearchRequest(baseUrl+"/search/?foo=%2F")) - for i := 0; i < 7; i++ { + for range 7 { requests = append(requests, createSearchRequest(baseUrl+"/search/")) } assertRequests(requests, client, t) diff --git a/client/go/internal/cli/cmd/visit_test.go b/client/go/internal/cli/cmd/visit_test.go index ae494059691..85594912da2 100644 --- a/client/go/internal/cli/cmd/visit_test.go +++ b/client/go/internal/cli/cmd/visit_test.go @@ -41,7 +41,7 @@ func TestQuoteFunc(t *testing.T) { var buf []byte = make([]byte, 3) buf[0] = 'a' buf[2] = 'z' - for i := 0; i < 256; i++ { + for i := range 256 { buf[1] = byte(i) s := string(buf) res := quoteArgForUrl(s) diff --git a/client/go/internal/vespa/document/document_test.go b/client/go/internal/vespa/document/document_test.go index 50698e3f8a3..8875ad83291 100644 --- a/client/go/internal/vespa/document/document_test.go +++ b/client/go/internal/vespa/document/document_test.go @@ -176,7 +176,7 @@ func testDocumentDecoder(t *testing.T, jsonLike string) { if len(docs) != len(result) { t.Errorf("len(result) = %d, want %d", len(result), len(docs)) } - for i := 0; i < len(docs); i++ { + for i := range len(docs) { got := result[i] want := docs[i] if !got.Equal(want) { diff --git a/client/go/internal/vespa/document/http.go b/client/go/internal/vespa/document/http.go index 800ac0a3887..df4d97e2a82 100644 --- a/client/go/internal/vespa/document/http.go +++ b/client/go/internal/vespa/document/http.go @@ -95,7 +95,7 @@ func NewClient(options ClientOptions, httpClients []httputil.Client) (*Client, e } c.gzippers.New = func() any { return gzip.NewWriter(io.Discard) } c.buffers.New = func() any { return &bytes.Buffer{} } - for i := 0; i < runtime.NumCPU(); i++ { + for range runtime.NumCPU() { go c.preparePending() } return c, nil diff --git a/client/go/internal/vespa/document/http_test.go b/client/go/internal/vespa/document/http_test.go index 74133fc73d8..878b7a98be3 100644 --- a/client/go/internal/vespa/document/http_test.go +++ b/client/go/internal/vespa/document/http_test.go @@ -34,7 +34,7 @@ type mockHTTPClient struct { func TestLeastBusyClient(t *testing.T) { httpClient := mock.HTTPClient{} var httpClients []httputil.Client - for i := 0; i < 4; i++ { + for i := range 4 { httpClients = append(httpClients, &mockHTTPClient{i, &httpClient}) } client, _ := NewClient(ClientOptions{}, httpClients) diff --git a/client/go/internal/vespa/document/throttler_test.go b/client/go/internal/vespa/document/throttler_test.go index eba8cbd2972..3cdecb22be4 100644 --- a/client/go/internal/vespa/document/throttler_test.go +++ b/client/go/internal/vespa/document/throttler_test.go @@ -13,7 +13,7 @@ func TestThrottler(t *testing.T) { if got, want := tr.TargetInflight(), int64(16); got != want { t.Errorf("got TargetInflight() = %d, but want %d", got, want) } - for i := 0; i < 65; i++ { + for range 65 { tr.Sent() tr.Success() } diff --git a/client/go/internal/vespa/load_env.go b/client/go/internal/vespa/load_env.go index 5cae03694bc..4eb1297e711 100644 --- a/client/go/internal/vespa/load_env.go +++ b/client/go/internal/vespa/load_env.go @@ -151,7 +151,7 @@ func nSpacedFields(s string, n int) []string { // pretty strict for now, can be more lenient if needed func isValidShellVariableName(s string) bool { - for i := 0; i < len(s); i++ { + for i := range len(s) { b := s[i] switch { case (b >= 'A' && b <= 'Z'): // ok diff --git a/client/go/internal/vespa/target_test.go b/client/go/internal/vespa/target_test.go index f886c9117a9..2b4cf485b83 100644 --- a/client/go/internal/vespa/target_test.go +++ b/client/go/internal/vespa/target_test.go @@ -20,7 +20,7 @@ func TestLocalTarget(t *testing.T) { client := &mock.HTTPClient{} lt := LocalTarget(client, TLSOptions{}, 0) assertServiceURL(t, "http://127.0.0.1:19071", lt, "deploy") - for i := 0; i < 2; i++ { + for range 2 { response := ` { "services": [ @@ -83,7 +83,7 @@ func TestCustomTargetWait(t *testing.T) { client.NextStatus(500) assertService(t, true, target, "", 0) // Fails multiple times - for i := 0; i < 3; i++ { + for range 3 { client.NextStatus(500) client.NextResponseError(io.EOF) } @@ -120,7 +120,7 @@ func TestCustomTargetAwaitDeployment(t *testing.T) { func TestCustomTargetCompatibleWith(t *testing.T) { client := &mock.HTTPClient{} target := CustomTarget(client, "http://192.0.2.42", TLSOptions{}, 0) - for i := 0; i < 3; i++ { + for range 3 { client.NextResponse(mock.HTTPResponse{ URI: "/state/v1/version", Status: 200, @@ -249,7 +249,7 @@ func TestLog(t *testing.T) { func TestCloudCompatibleWith(t *testing.T) { target, client := createCloudTarget(t, io.Discard) - for i := 0; i < 3; i++ { + for range 3 { client.NextResponse(mock.HTTPResponse{URI: "/cli/v1/", Status: 200, Body: []byte(`{"minVersion":"8.0.0"}`)}) } assert.Nil(t, target.CompatibleWith(version.MustParse("8.0.0"))) diff --git a/config-model/src/main/java/com/yahoo/vespa/model/admin/otel/OpenTelemetryConfigGenerator.java b/config-model/src/main/java/com/yahoo/vespa/model/admin/otel/OpenTelemetryConfigGenerator.java index 9bac6e4553d..67c45d58a95 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/admin/otel/OpenTelemetryConfigGenerator.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/admin/otel/OpenTelemetryConfigGenerator.java @@ -295,7 +295,7 @@ public class OpenTelemetryConfigGenerator { { g.writeFieldName("exporters"); g.writeStartArray(); - g.writeString("file"); + g.writeString("otlphttp/gw"); g.writeEndArray(); } g.writeEndObject(); // metrics diff --git a/config/src/tests/failover/failover.cpp b/config/src/tests/failover/failover.cpp index 80d89f41c16..a679b2a0ecd 100644 --- a/config/src/tests/failover/failover.cpp +++ b/config/src/tests/failover/failover.cpp @@ -10,6 +10,7 @@ #include "config-my.h" #include <vespa/vespalib/data/slime/slime.h> #include <vespa/vespalib/data/simple_buffer.h> +#include <vespa/vespalib/util/barrier.h> #include <vespa/log/log.h> LOG_SETUP("failover"); diff --git a/dependency-versions/pom.xml b/dependency-versions/pom.xml index 747c97d31eb..f172b09b7fe 100644 --- a/dependency-versions/pom.xml +++ b/dependency-versions/pom.xml @@ -147,8 +147,8 @@ <surefire.vespa.version>3.3.0</surefire.vespa.version> <velocity.vespa.version>2.3</velocity.vespa.version> <velocity.tools.vespa.version>3.1</velocity.tools.vespa.version> - <wiremock.vespa.version>3.6.0</wiremock.vespa.version> - <woodstox.vespa.version>6.6.2</woodstox.vespa.version> + <wiremock.vespa.version>3.7.0</wiremock.vespa.version> + <woodstox.vespa.version>7.0.0</woodstox.vespa.version> <stax2-api.vespa.version>4.2.2</stax2-api.vespa.version> <xerces.vespa.version>2.12.2</xerces.vespa.version> <zero-allocation-hashing.vespa.version>0.16</zero-allocation-hashing.vespa.version> @@ -171,7 +171,7 @@ <maven-bundle-plugin.vespa.version>5.1.9</maven-bundle-plugin.vespa.version> <maven-compiler-plugin.vespa.version>3.13.0</maven-compiler-plugin.vespa.version> <maven-core.vespa.version>3.9.8</maven-core.vespa.version> - <maven-dependency-plugin.vespa.version>3.7.0</maven-dependency-plugin.vespa.version> + <maven-dependency-plugin.vespa.version>3.7.1</maven-dependency-plugin.vespa.version> <maven-deploy-plugin.vespa.version>3.1.2</maven-deploy-plugin.vespa.version> <maven-enforcer-plugin.vespa.version>3.5.0</maven-enforcer-plugin.vespa.version> <maven-failsafe-plugin.vespa.version>3.3.0</maven-failsafe-plugin.vespa.version> diff --git a/document/src/tests/documentupdatetestcase.cpp b/document/src/tests/documentupdatetestcase.cpp index 9da03d6001b..7c571df257c 100644 --- a/document/src/tests/documentupdatetestcase.cpp +++ b/document/src/tests/documentupdatetestcase.cpp @@ -33,8 +33,11 @@ #include <vespa/eval/eval/value.h> #include <vespa/vespalib/gtest/gtest.h> #include <vespa/vespalib/objects/nbostream.h> +#include <vespa/vespalib/test/test_data.h> +#include <vespa/vespalib/testkit/test_path.h> #include <vespa/vespalib/util/exception.h> #include <vespa/vespalib/util/exceptions.h> +#include <filesystem> #include <fstream> #include <unistd.h> @@ -44,6 +47,7 @@ using vespalib::eval::SimpleValue; using vespalib::eval::TensorSpec; using vespalib::eval::ValueType; using vespalib::nbostream; +using vespalib::test::TestDataBase; namespace document { @@ -87,31 +91,38 @@ void testRoundtripSerialize(const UpdateType& update, const DataType &type) { } } -void -writeBufferToFile(const nbostream &buf, const vespalib::string &fileName) +} + +class DocumentUpdateTest : public ::testing::Test, public vespalib::test::TestData<DocumentUpdateTest> { +protected: + DocumentUpdateTest(); + ~DocumentUpdateTest() override; + static void SetUpTestSuite(); + static void TearDownTestSuite(); +}; + +DocumentUpdateTest::DocumentUpdateTest() + : ::testing::Test(), + vespalib::test::TestData<DocumentUpdateTest>() { - auto file = std::fstream(fileName, std::ios::out | std::ios::binary); - file.write(buf.data(), buf.size()); - assert(file.good()); - file.close(); } -nbostream -readBufferFromFile(const vespalib::string &fileName) +DocumentUpdateTest::~DocumentUpdateTest() = default; + +void +DocumentUpdateTest::SetUpTestSuite() { - auto file = std::fstream(fileName, std::ios::in | std::ios::binary | std::ios::ate); - auto size = file.tellg(); - file.seekg(0); - vespalib::alloc::Alloc buf = vespalib::alloc::Alloc::alloc(size); - file.read(static_cast<char *>(buf.get()), size); - assert(file.good()); - file.close(); - return nbostream(std::move(buf), size); + setup_test_data(TEST_PATH("data"), "documentupdate-build-data"); + std::filesystem::create_directory(build_testdata()); } +void +DocumentUpdateTest::TearDownTestSuite() +{ + tear_down_test_data(); } -TEST(DocumentUpdateTest, testSimpleUsage) +TEST_F(DocumentUpdateTest, testSimpleUsage) { DocumenttypesConfigBuilderHelper builder; builder.document(42, "test", @@ -205,7 +216,7 @@ TEST(DocumentUpdateTest, testSimpleUsage) } } -TEST(DocumentUpdateTest, testClearField) +TEST_F(DocumentUpdateTest, testClearField) { // Create a document. TestDocMan docMan; @@ -220,7 +231,7 @@ TEST(DocumentUpdateTest, testClearField) EXPECT_FALSE(doc->getValue("headerval")); } -TEST(DocumentUpdateTest, testUpdateApplySingleValue) +TEST_F(DocumentUpdateTest, testUpdateApplySingleValue) { // Create a document. TestDocMan docMan; @@ -235,7 +246,7 @@ TEST(DocumentUpdateTest, testUpdateApplySingleValue) EXPECT_EQ(9, doc->getValue("headerval")->getAsInt()); } -TEST(DocumentUpdateTest, testUpdateArray) +TEST_F(DocumentUpdateTest, testUpdateArray) { // Create a document. TestDocMan docMan; @@ -314,7 +325,7 @@ createAddUpdate(int key, int weight) { return upd; } -TEST(DocumentUpdateTest, testUpdateWeightedSet) +TEST_F(DocumentUpdateTest, testUpdateWeightedSet) { // Create a test document TestDocMan docMan; @@ -427,7 +438,7 @@ WeightedSetAutoCreateFixture::WeightedSetAutoCreateFixture() } } // anon ns -TEST(DocumentUpdateTest, testIncrementNonExistingAutoCreateWSetField) +TEST_F(DocumentUpdateTest, testIncrementNonExistingAutoCreateWSetField) { WeightedSetAutoCreateFixture fixture; @@ -440,7 +451,7 @@ TEST(DocumentUpdateTest, testIncrementNonExistingAutoCreateWSetField) EXPECT_EQ(1, ws->get(StringFieldValue("foo"), 0)); } -TEST(DocumentUpdateTest, testIncrementExistingWSetField) +TEST_F(DocumentUpdateTest, testIncrementExistingWSetField) { WeightedSetAutoCreateFixture fixture; { @@ -456,7 +467,7 @@ TEST(DocumentUpdateTest, testIncrementExistingWSetField) EXPECT_EQ(1, ws->get(StringFieldValue("foo"), 0)); } -TEST(DocumentUpdateTest, testIncrementWithZeroResultWeightIsRemoved) +TEST_F(DocumentUpdateTest, testIncrementWithZeroResultWeightIsRemoved) { WeightedSetAutoCreateFixture fixture; fixture.update.addUpdate(FieldUpdate(fixture.field) @@ -471,13 +482,13 @@ TEST(DocumentUpdateTest, testIncrementWithZeroResultWeightIsRemoved) EXPECT_FALSE(ws->contains(StringFieldValue("baz"))); } -TEST(DocumentUpdateTest, testReadSerializedFile) +TEST_F(DocumentUpdateTest, testReadSerializedFile) { // Reads a file serialized from java - const std::string file_name = "data/crossplatform-java-cpp-doctypes.cfg"; + const std::string file_name = source_testdata() + "/crossplatform-java-cpp-doctypes.cfg"; DocumentTypeRepo repo(readDocumenttypesConfig(file_name)); - auto is = readBufferFromFile("data/serializeupdatejava.dat"); + auto is = read_buffer_from_file(source_testdata() + "/serializeupdatejava.dat"); DocumentUpdate::UP updp(DocumentUpdate::createHEAD(repo, is)); DocumentUpdate& upd(*updp); @@ -533,11 +544,11 @@ TEST(DocumentUpdateTest, testReadSerializedFile) } -TEST(DocumentUpdateTest, testGenerateSerializedFile) +TEST_F(DocumentUpdateTest, testGenerateSerializedFile) { // Tests nothing, only generates a file for java test - const std::string file_name = "data/crossplatform-java-cpp-doctypes.cfg"; - DocumentTypeRepo repo(readDocumenttypesConfig(file_name)); + const std::string cfg_file_name = source_testdata() + "/crossplatform-java-cpp-doctypes.cfg"; + DocumentTypeRepo repo(readDocumenttypesConfig(cfg_file_name)); const DocumentType *type(repo.getDocumentType("serializetest")); DocumentUpdate upd(repo, *type, DocumentId("id:ns:serializetest::update")); @@ -557,11 +568,13 @@ TEST(DocumentUpdateTest, testGenerateSerializedFile) .addUpdate(std::make_unique<MapValueUpdate>(StringFieldValue::make("foo"), std::make_unique<ArithmeticValueUpdate>(ArithmeticValueUpdate::Mul, 2)))); nbostream buf(serializeHEAD(upd)); - writeBufferToFile(buf, "data/serializeupdatecpp.dat"); + std::string file_name("serializeupdatecpp.dat"); + write_buffer_to_file(buf, build_testdata() + "/" + file_name); + ASSERT_NO_FATAL_FAILURE(remove_unchanged_build_testdata_file_or_fail(buf, file_name)); } -TEST(DocumentUpdateTest, testSetBadFieldTypes) +TEST_F(DocumentUpdateTest, testSetBadFieldTypes) { // Create a test document TestDocMan docMan; @@ -582,7 +595,7 @@ TEST(DocumentUpdateTest, testSetBadFieldTypes) doc->getValue(doc->getField("headerval")).get()); } -TEST(DocumentUpdateTest, testUpdateApplyNoParams) +TEST_F(DocumentUpdateTest, testUpdateApplyNoParams) { TestDocMan docMan; Document::UP doc(docMan.createDocument()); @@ -597,7 +610,7 @@ TEST(DocumentUpdateTest, testUpdateApplyNoParams) EXPECT_FALSE(doc->hasValue(doc->getField("tags"))); } -TEST(DocumentUpdateTest, testUpdateApplyNoArrayValues) +TEST_F(DocumentUpdateTest, testUpdateApplyNoArrayValues) { TestDocMan docMan; Document::UP doc(docMan.createDocument()); @@ -617,7 +630,7 @@ TEST(DocumentUpdateTest, testUpdateApplyNoArrayValues) EXPECT_EQ((size_t) 0, fval->size()); } -TEST(DocumentUpdateTest, testUpdateArrayEmptyParamValue) +TEST_F(DocumentUpdateTest, testUpdateArrayEmptyParamValue) { // Create a test document. TestDocMan docMan; @@ -645,7 +658,7 @@ TEST(DocumentUpdateTest, testUpdateArrayEmptyParamValue) EXPECT_FALSE(fval2); } -TEST(DocumentUpdateTest, testUpdateWeightedSetEmptyParamValue) +TEST_F(DocumentUpdateTest, testUpdateWeightedSetEmptyParamValue) { // Create a test document TestDocMan docMan; @@ -673,7 +686,7 @@ TEST(DocumentUpdateTest, testUpdateWeightedSetEmptyParamValue) EXPECT_FALSE(fval2); } -TEST(DocumentUpdateTest, testUpdateArrayWrongSubtype) +TEST_F(DocumentUpdateTest, testUpdateArrayWrongSubtype) { // Create a test document TestDocMan docMan; @@ -697,7 +710,7 @@ TEST(DocumentUpdateTest, testUpdateArrayWrongSubtype) EXPECT_EQ((document::FieldValue*) 0, fval.get()); } -TEST(DocumentUpdateTest, testUpdateWeightedSetWrongSubtype) +TEST_F(DocumentUpdateTest, testUpdateWeightedSetWrongSubtype) { // Create a test document TestDocMan docMan; @@ -721,7 +734,7 @@ TEST(DocumentUpdateTest, testUpdateWeightedSetWrongSubtype) EXPECT_EQ((document::FieldValue*) 0, fval.get()); } -TEST(DocumentUpdateTest, testMapValueUpdate) +TEST_F(DocumentUpdateTest, testMapValueUpdate) { // Create a test document TestDocMan docMan; @@ -940,7 +953,7 @@ struct TensorUpdateFixture { }; -TEST(DocumentUpdateTest, tensor_assign_update_can_be_applied) +TEST_F(DocumentUpdateTest, tensor_assign_update_can_be_applied) { TensorUpdateFixture f; f.applyUpdate(std::make_unique<AssignValueUpdate>(f.makeBaselineTensor())); @@ -948,7 +961,7 @@ TEST(DocumentUpdateTest, tensor_assign_update_can_be_applied) f.assertTensor(*f.makeBaselineTensor()); } -TEST(DocumentUpdateTest, tensor_clear_update_can_be_applied) +TEST_F(DocumentUpdateTest, tensor_clear_update_can_be_applied) { TensorUpdateFixture f; f.setTensor(*f.makeBaselineTensor()); @@ -957,7 +970,7 @@ TEST(DocumentUpdateTest, tensor_clear_update_can_be_applied) EXPECT_FALSE(f.getTensor()); } -TEST(DocumentUpdateTest, tensor_add_update_can_be_applied) +TEST_F(DocumentUpdateTest, tensor_add_update_can_be_applied) { TensorUpdateFixture f; f.assertApplyUpdate(f.spec().add({{"x", "a"}}, 2) @@ -971,7 +984,7 @@ TEST(DocumentUpdateTest, tensor_add_update_can_be_applied) .add({{"x", "c"}}, 7)); } -TEST(DocumentUpdateTest, tensor_add_update_can_be_applied_to_nonexisting_tensor) +TEST_F(DocumentUpdateTest, tensor_add_update_can_be_applied_to_nonexisting_tensor) { TensorUpdateFixture f; f.assertApplyUpdateNonExisting(std::make_unique<TensorAddUpdate>(f.makeTensor(f.spec().add({{"x", "b"}}, 5) @@ -981,7 +994,7 @@ TEST(DocumentUpdateTest, tensor_add_update_can_be_applied_to_nonexisting_tensor) .add({{"x", "c"}}, 7)); } -TEST(DocumentUpdateTest, tensor_remove_update_can_be_applied) +TEST_F(DocumentUpdateTest, tensor_remove_update_can_be_applied) { TensorUpdateFixture f; f.assertApplyUpdate(f.spec().add({{"x", "a"}}, 2) @@ -992,13 +1005,13 @@ TEST(DocumentUpdateTest, tensor_remove_update_can_be_applied) f.spec().add({{"x", "a"}}, 2)); } -TEST(DocumentUpdateTest, tensor_remove_update_can_be_applied_to_nonexisting_tensor) +TEST_F(DocumentUpdateTest, tensor_remove_update_can_be_applied_to_nonexisting_tensor) { TensorUpdateFixture f; f.assertApplyUpdateNonExisting(std::make_unique<TensorRemoveUpdate>(f.makeTensor(f.spec().add({{"x", "b"}}, 1)))); } -TEST(DocumentUpdateTest, tensor_modify_update_can_be_applied) +TEST_F(DocumentUpdateTest, tensor_modify_update_can_be_applied) { TensorUpdateFixture f; auto baseLine = f.spec().add({{"x", "a"}}, 2) @@ -1024,7 +1037,7 @@ TEST(DocumentUpdateTest, tensor_modify_update_can_be_applied) .add({{"x", "b"}}, 15)); } -TEST(DocumentUpdateTest, tensor_modify_update_with_create_non_existing_cells_can_be_applied) +TEST_F(DocumentUpdateTest, tensor_modify_update_with_create_non_existing_cells_can_be_applied) { TensorUpdateFixture f; auto baseLine = f.spec().add({{"x", "a"}}, 2) @@ -1038,14 +1051,14 @@ TEST(DocumentUpdateTest, tensor_modify_update_with_create_non_existing_cells_can .add({{"x", "c"}}, 6)); } -TEST(DocumentUpdateTest, tensor_modify_update_is_ignored_when_applied_to_nonexisting_tensor) +TEST_F(DocumentUpdateTest, tensor_modify_update_is_ignored_when_applied_to_nonexisting_tensor) { TensorUpdateFixture f; f.assertApplyUpdateNonExisting(std::make_unique<TensorModifyUpdate>(TensorModifyUpdate::Operation::ADD, f.makeTensor(f.spec().add({{"x", "b"}}, 5)))); } -TEST(DocumentUpdateTest, tensor_modify_update_with_create_non_existing_cells_is_applied_to_nonexisting_tensor) +TEST_F(DocumentUpdateTest, tensor_modify_update_with_create_non_existing_cells_is_applied_to_nonexisting_tensor) { TensorUpdateFixture f; f.assertApplyUpdateNonExisting(std::make_unique<TensorModifyUpdate>(TensorModifyUpdate::Operation::ADD, @@ -1055,25 +1068,25 @@ TEST(DocumentUpdateTest, tensor_modify_update_with_create_non_existing_cells_is_ .add({{"x", "c"}}, 6)); } -TEST(DocumentUpdateTest, tensor_assign_update_can_be_roundtrip_serialized) +TEST_F(DocumentUpdateTest, tensor_assign_update_can_be_roundtrip_serialized) { TensorUpdateFixture f; f.assertRoundtripSerialize(AssignValueUpdate(f.makeBaselineTensor())); } -TEST(DocumentUpdateTest, tensor_add_update_can_be_roundtrip_serialized) +TEST_F(DocumentUpdateTest, tensor_add_update_can_be_roundtrip_serialized) { TensorUpdateFixture f; f.assertRoundtripSerialize(TensorAddUpdate(f.makeBaselineTensor())); } -TEST(DocumentUpdateTest, tensor_remove_update_can_be_roundtrip_serialized) +TEST_F(DocumentUpdateTest, tensor_remove_update_can_be_roundtrip_serialized) { TensorUpdateFixture f; f.assertRoundtripSerialize(TensorRemoveUpdate(f.makeBaselineTensor())); } -TEST(DocumentUpdateTest, tensor_remove_update_with_not_fully_specified_address_can_be_roundtrip_serialized) +TEST_F(DocumentUpdateTest, tensor_remove_update_with_not_fully_specified_address_can_be_roundtrip_serialized) { TensorUpdateFixture f("sparse_xy_tensor"); TensorDataType type(ValueType::from_spec("tensor(y{})")); @@ -1081,13 +1094,13 @@ TEST(DocumentUpdateTest, tensor_remove_update_with_not_fully_specified_address_c makeTensorFieldValue(TensorSpec("tensor(y{})").add({{"y", "a"}}, 1), type))); } -TEST(DocumentUpdateTest, tensor_remove_update_on_float_tensor_can_be_roundtrip_serialized) +TEST_F(DocumentUpdateTest, tensor_remove_update_on_float_tensor_can_be_roundtrip_serialized) { TensorUpdateFixture f("sparse_float_tensor"); f.assertRoundtripSerialize(TensorRemoveUpdate(f.makeBaselineTensor())); } -TEST(DocumentUpdateTest, tensor_modify_update_can_be_roundtrip_serialized) +TEST_F(DocumentUpdateTest, tensor_modify_update_can_be_roundtrip_serialized) { TensorUpdateFixture f; f.assertRoundtripSerialize(TensorModifyUpdate(TensorModifyUpdate::Operation::REPLACE, f.makeBaselineTensor())); @@ -1098,7 +1111,7 @@ TEST(DocumentUpdateTest, tensor_modify_update_can_be_roundtrip_serialized) f.assertRoundtripSerialize(TensorModifyUpdate(TensorModifyUpdate::Operation::MULTIPLY, f.makeBaselineTensor(), 1.0)); } -TEST(DocumentUpdateTest, tensor_modify_update_on_float_tensor_can_be_roundtrip_serialized) +TEST_F(DocumentUpdateTest, tensor_modify_update_on_float_tensor_can_be_roundtrip_serialized) { TensorUpdateFixture f("sparse_float_tensor"); f.assertRoundtripSerialize(TensorModifyUpdate(TensorModifyUpdate::Operation::REPLACE, f.makeBaselineTensor())); @@ -1109,7 +1122,7 @@ TEST(DocumentUpdateTest, tensor_modify_update_on_float_tensor_can_be_roundtrip_s f.assertRoundtripSerialize(TensorModifyUpdate(TensorModifyUpdate::Operation::MULTIPLY, f.makeBaselineTensor(), 1.0)); } -TEST(DocumentUpdateTest, tensor_modify_update_on_dense_tensor_can_be_roundtrip_serialized) +TEST_F(DocumentUpdateTest, tensor_modify_update_on_dense_tensor_can_be_roundtrip_serialized) { TensorUpdateFixture f("dense_tensor"); vespalib::string sparseType("tensor(x{})"); @@ -1118,25 +1131,25 @@ TEST(DocumentUpdateTest, tensor_modify_update_on_dense_tensor_can_be_roundtrip_s f.assertRoundtripSerialize(TensorModifyUpdate(TensorModifyUpdate::Operation::REPLACE, std::move(sparseTensor))); } -TEST(DocumentUpdateTest, tensor_add_update_throws_on_non_tensor_field) +TEST_F(DocumentUpdateTest, tensor_add_update_throws_on_non_tensor_field) { TensorUpdateFixture f; f.assertThrowOnNonTensorField(TensorAddUpdate(f.makeBaselineTensor())); } -TEST(DocumentUpdateTest, tensor_remove_update_throws_on_non_tensor_field) +TEST_F(DocumentUpdateTest, tensor_remove_update_throws_on_non_tensor_field) { TensorUpdateFixture f; f.assertThrowOnNonTensorField(TensorRemoveUpdate(f.makeBaselineTensor())); } -TEST(DocumentUpdateTest, tensor_modify_update_throws_on_non_tensor_field) +TEST_F(DocumentUpdateTest, tensor_modify_update_throws_on_non_tensor_field) { TensorUpdateFixture f; f.assertThrowOnNonTensorField(TensorModifyUpdate(TensorModifyUpdate::Operation::REPLACE, f.makeBaselineTensor())); } -TEST(DocumentUpdateTest, tensor_remove_update_throws_if_address_tensor_is_not_sparse) +TEST_F(DocumentUpdateTest, tensor_remove_update_throws_if_address_tensor_is_not_sparse) { TensorUpdateFixture f("dense_tensor"); auto addressTensor = f.makeTensor(f.spec().add({{"x", 0}}, 2)); // creates a dense address tensor @@ -1145,7 +1158,7 @@ TEST(DocumentUpdateTest, tensor_remove_update_throws_if_address_tensor_is_not_sp vespalib::IllegalStateException); } -TEST(DocumentUpdateTest, tensor_modify_update_throws_if_cells_tensor_is_not_sparse) +TEST_F(DocumentUpdateTest, tensor_modify_update_throws_if_cells_tensor_is_not_sparse) { TensorUpdateFixture f("dense_tensor"); auto cellsTensor = f.makeTensor(f.spec().add({{"x", 0}}, 2)); // creates a dense cells tensor @@ -1209,31 +1222,34 @@ struct TensorUpdateSerializeFixture { void serializeUpdateToFile(const DocumentUpdate &update, const vespalib::string &fileName) { nbostream buf = serializeHEAD(update); - writeBufferToFile(buf, fileName); + TestDataBase::write_buffer_to_file(buf, fileName); } DocumentUpdate::UP deserializeUpdateFromFile(const vespalib::string &fileName) { - auto stream = readBufferFromFile(fileName); + auto stream = TestDataBase::read_buffer_from_file(fileName); return DocumentUpdate::createHEAD(*repo, stream); } }; -TEST(DocumentUpdateTest, tensor_update_file_java_can_be_deserialized) +TEST_F(DocumentUpdateTest, tensor_update_file_java_can_be_deserialized) { TensorUpdateSerializeFixture f; - auto update = f.deserializeUpdateFromFile("data/serialize-tensor-update-java.dat"); + auto update = f.deserializeUpdateFromFile(source_testdata() + "/serialize-tensor-update-java.dat"); EXPECT_EQ(*f.makeUpdate(), *update); } -TEST(DocumentUpdateTest, generate_serialized_tensor_update_file_cpp) +TEST_F(DocumentUpdateTest, generate_serialized_tensor_update_file_cpp) { TensorUpdateSerializeFixture f; auto update = f.makeUpdate(); - f.serializeUpdateToFile(*update, "data/serialize-tensor-update-cpp.dat"); + std::string file_name("serialize-tensor-update-cpp.dat"); + auto act_path = build_testdata() + "/" + file_name; + f.serializeUpdateToFile(*update, act_path); + auto buf = read_buffer_from_file(act_path); + ASSERT_NO_FATAL_FAILURE(remove_unchanged_build_testdata_file_or_fail(buf, file_name)); } - void assertDocumentUpdateFlag(bool createIfNonExistent, int value) { @@ -1249,7 +1265,7 @@ assertDocumentUpdateFlag(bool createIfNonExistent, int value) EXPECT_EQ(value, extractedValue); } -TEST(DocumentUpdateTest, testThatDocumentUpdateFlagsIsWorking) +TEST_F(DocumentUpdateTest, testThatDocumentUpdateFlagsIsWorking) { { // create-if-non-existent = true assertDocumentUpdateFlag(true, 0); @@ -1289,7 +1305,7 @@ CreateIfNonExistentFixture::CreateIfNonExistentFixture() update->setCreateIfNonExistent(true); } -TEST(DocumentUpdateTest, testThatCreateIfNonExistentFlagIsSerializedAndDeserialized) +TEST_F(DocumentUpdateTest, testThatCreateIfNonExistentFlagIsSerializedAndDeserialized) { CreateIfNonExistentFixture f; @@ -1322,7 +1338,7 @@ ArrayUpdateFixture::ArrayUpdateFixture() } ArrayUpdateFixture::~ArrayUpdateFixture() = default; -TEST(DocumentUpdateTest, array_element_update_can_be_roundtrip_serialized) +TEST_F(DocumentUpdateTest, array_element_update_can_be_roundtrip_serialized) { ArrayUpdateFixture f; @@ -1332,7 +1348,7 @@ TEST(DocumentUpdateTest, array_element_update_can_be_roundtrip_serialized) EXPECT_EQ(*f.update, *deserialized); } -TEST(DocumentUpdateTest, array_element_update_applies_to_specified_element) +TEST_F(DocumentUpdateTest, array_element_update_applies_to_specified_element) { ArrayUpdateFixture f; @@ -1351,7 +1367,7 @@ TEST(DocumentUpdateTest, array_element_update_applies_to_specified_element) EXPECT_EQ(vespalib::string("blarg"), (*result_array)[2].getAsString()); } -TEST(DocumentUpdateTest, array_element_update_for_invalid_index_is_ignored) +TEST_F(DocumentUpdateTest, array_element_update_for_invalid_index_is_ignored) { ArrayUpdateFixture f; @@ -1414,7 +1430,7 @@ struct UpdateToEmptyDocumentFixture { } }; -TEST(DocumentUpdateTest, string_field_annotations_can_be_deserialized_after_assign_update_to_empty_document) +TEST_F(DocumentUpdateTest, string_field_annotations_can_be_deserialized_after_assign_update_to_empty_document) { UpdateToEmptyDocumentFixture f; auto doc = f.make_empty_doc(); diff --git a/fnet/src/tests/locking/lockspeed.cpp b/fnet/src/tests/locking/lockspeed.cpp index 78fe0869e22..eaf79ff38d4 100644 --- a/fnet/src/tests/locking/lockspeed.cpp +++ b/fnet/src/tests/locking/lockspeed.cpp @@ -2,6 +2,7 @@ #include <vespa/vespalib/testkit/test_kit.h> #include "dummy.h" #include <chrono> +#include <condition_variable> class SpinLock { private: @@ -202,16 +203,16 @@ TEST("lock speed") { start = clock::now(); for (i = 0; i < 1000000; i++) { - DummyObj *dummy0 = new DummyObj(); - DummyObj *dummy1 = new DummyObj(); - DummyObj *dummy2 = new DummyObj(); - DummyObj *dummy3 = new DummyObj(); - DummyObj *dummy4 = new DummyObj(); - DummyObj *dummy5 = new DummyObj(); - DummyObj *dummy6 = new DummyObj(); - DummyObj *dummy7 = new DummyObj(); - DummyObj *dummy8 = new DummyObj(); - DummyObj *dummy9 = new DummyObj(); + auto *dummy0 = new DummyObj(); + auto *dummy1 = new DummyObj(); + auto *dummy2 = new DummyObj(); + auto *dummy3 = new DummyObj(); + auto *dummy4 = new DummyObj(); + auto *dummy5 = new DummyObj(); + auto *dummy6 = new DummyObj(); + auto *dummy7 = new DummyObj(); + auto *dummy8 = new DummyObj(); + auto *dummy9 = new DummyObj(); delete dummy9; delete dummy8; delete dummy7; diff --git a/fnet/src/tests/scheduling/schedule.cpp b/fnet/src/tests/scheduling/schedule.cpp index 88b84bec67a..2890cc1d5f0 100644 --- a/fnet/src/tests/scheduling/schedule.cpp +++ b/fnet/src/tests/scheduling/schedule.cpp @@ -2,6 +2,7 @@ #include <vespa/vespalib/testkit/test_kit.h> #include <vespa/fnet/scheduler.h> #include <vespa/fnet/task.h> +#include <cassert> using vespalib::steady_clock; using vespalib::steady_time; diff --git a/indexinglanguage/src/main/java/com/yahoo/vespa/indexinglanguage/expressions/EmbedExpression.java b/indexinglanguage/src/main/java/com/yahoo/vespa/indexinglanguage/expressions/EmbedExpression.java index 05ac73618e8..c2d41edf1b6 100644 --- a/indexinglanguage/src/main/java/com/yahoo/vespa/indexinglanguage/expressions/EmbedExpression.java +++ b/indexinglanguage/src/main/java/com/yahoo/vespa/indexinglanguage/expressions/EmbedExpression.java @@ -187,8 +187,8 @@ public class EmbedExpression extends Expression { if (targetType.rank() == 2 && targetType.mappedSubtype().rank() == 2) { if (embedderArguments.size() != 1) throw new VerificationException(this, "When the embedding target field is a 2d mapped tensor " + - "the name of the tensor dimension that corresponds to the input array elements must " + - "be given as a second argument to embed, e.g: ... | embed splade paragraph | ..."); + "the name of the tensor dimension that corresponds to the input array elements must " + + "be given as a second argument to embed, e.g: ... | embed splade paragraph | ..."); if ( ! targetType.mappedSubtype().dimensionNames().contains(embedderArguments.get(0))) { throw new VerificationException(this, "The dimension '" + embedderArguments.get(0) + "' given to embed " + "is not a sparse dimension of the target type " + targetType); @@ -254,7 +254,7 @@ public class EmbedExpression extends Expression { List<String> embedderIds = new ArrayList<>(); embedders.forEach((key, value) -> embedderIds.add(key)); embedderIds.sort(null); - return String.join(",", embedderIds); + return String.join(", ", embedderIds); } } diff --git a/indexinglanguage/src/test/java/com/yahoo/vespa/indexinglanguage/ScriptTestCase.java b/indexinglanguage/src/test/java/com/yahoo/vespa/indexinglanguage/ScriptTestCase.java index f6995ac5a72..dd0ec255c35 100644 --- a/indexinglanguage/src/test/java/com/yahoo/vespa/indexinglanguage/ScriptTestCase.java +++ b/indexinglanguage/src/test/java/com/yahoo/vespa/indexinglanguage/ScriptTestCase.java @@ -202,9 +202,9 @@ public class ScriptTestCase { "my input", "[110.0, 122.0, 33.0, 106.0]"); assertThrows(() -> testEmbedStatement("input myText | embed | attribute 'myTensor'", embedders, "input text", "[105, 110, 112, 117]"), - "Multiple embedders are provided but no embedder id is given. Valid embedders are emb1,emb2"); + "Multiple embedders are provided but no embedder id is given. Valid embedders are emb1, emb2"); assertThrows(() -> testEmbedStatement("input myText | embed emb3 | attribute 'myTensor'", embedders, "input text", "[105, 110, 112, 117]"), - "Can't find embedder 'emb3'. Valid embedders are emb1,emb2"); + "Can't find embedder 'emb3'. Valid embedders are emb1, emb2"); } private void testEmbedStatement(String expressionString, Map<String, Embedder> embedders, String input, String expected) { @@ -562,12 +562,12 @@ public class ScriptTestCase { } - private void assertThrows(Runnable r, String msg) { + private void assertThrows(Runnable r, String expectedMessage) { try { r.run(); fail(); } catch (IllegalStateException e) { - assertEquals(e.getMessage(), msg); + assertEquals(expectedMessage, e.getMessage()); } } diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/NodeMetricsDbMaintainer.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/NodeMetricsDbMaintainer.java index 0ff8acdcf1f..7f13e3a5bca 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/NodeMetricsDbMaintainer.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/NodeMetricsDbMaintainer.java @@ -2,6 +2,7 @@ package com.yahoo.vespa.hosted.provision.maintenance; import com.yahoo.config.provision.ApplicationId; +import com.yahoo.config.provision.Environment; import com.yahoo.jdisc.Metric; import com.yahoo.lang.MutableInteger; import com.yahoo.vespa.hosted.provision.NodeRepository; @@ -75,9 +76,10 @@ public class NodeMetricsDbMaintainer extends NodeRepositoryMaintainer { MutableInteger failures, ApplicationId application) { if (exception != null) { - if (failures.get() < maxWarningsPerInvocation) + if (nodeRepository().zone().environment() == Environment.prod + && failures.get() < maxWarningsPerInvocation) log.log(Level.WARNING, "Could not update metrics for " + application + ": " + - Exceptions.toMessageString(exception)); + Exceptions.toMessageString(exception)); failures.add(1); } else if (response != null) { diff --git a/renovate.json b/renovate.json index b815ae4bf88..6e992909ba0 100644 --- a/renovate.json +++ b/renovate.json @@ -21,7 +21,6 @@ { "description": "Disable automatic PRs for artifacts, e.g. fixed version required like ZK dependencies or released to frequently. PRs can still be created manually from dependency dashboard.", "matchPackageNames": [ - "github.com/go-json-experiment/json", "javax.servlet:javax.servlet-api", "io.dropwizard.metrics:metrics-core", "org.apache.zookeeper:zookeeper" diff --git a/searchcore/CMakeLists.txt b/searchcore/CMakeLists.txt index 1d4019a170e..6a5d3915d6a 100644 --- a/searchcore/CMakeLists.txt +++ b/searchcore/CMakeLists.txt @@ -68,11 +68,8 @@ vespa_define_module( src/tests/proton/bucketdb/bucketdb src/tests/proton/common src/tests/proton/common/alloc_config - src/tests/proton/common/attribute_updater - src/tests/proton/common/document_type_inspector src/tests/proton/common/hw_info_sampler src/tests/proton/common/operation_rate_tracker - src/tests/proton/common/state_reporter_utils src/tests/proton/common/timer src/tests/proton/docsummary src/tests/proton/document_iterator @@ -101,7 +98,6 @@ vespa_define_module( src/tests/proton/documentmetastore/lid_allocator src/tests/proton/documentmetastore/lid_state_vector src/tests/proton/feed_and_search - src/tests/proton/feedoperation src/tests/proton/feedtoken src/tests/proton/flushengine src/tests/proton/flushengine/prepare_restart_flush_strategy @@ -121,9 +117,6 @@ vespa_define_module( src/tests/proton/matching/request_context src/tests/proton/matching/same_element_builder src/tests/proton/matching/unpacking_iterators_optimizer - src/tests/proton/metrics/documentdb_job_trackers - src/tests/proton/metrics/job_load_sampler - src/tests/proton/metrics/job_tracked_flush src/tests/proton/metrics/metrics_engine src/tests/proton/persistenceconformance src/tests/proton/persistenceengine @@ -152,7 +145,6 @@ vespa_define_module( src/tests/proton/server/memory_flush_config_updater src/tests/proton/server/memoryflush src/tests/proton/server/shared_threading_service - src/tests/proton/statusreport src/tests/proton/summaryengine src/tests/proton/verify_ranksetup src/tests/index/disk_indexes diff --git a/searchcore/src/tests/proton/common/.gitignore b/searchcore/src/tests/proton/common/.gitignore index 9ce51ef2178..e69de29bb2d 100644 --- a/searchcore/src/tests/proton/common/.gitignore +++ b/searchcore/src/tests/proton/common/.gitignore @@ -1,2 +0,0 @@ -searchcore_cachedselect_test_app -searchcore_selectpruner_test_app diff --git a/searchcore/src/tests/proton/common/CMakeLists.txt b/searchcore/src/tests/proton/common/CMakeLists.txt index 658afa38247..7eec733214b 100644 --- a/searchcore/src/tests/proton/common/CMakeLists.txt +++ b/searchcore/src/tests/proton/common/CMakeLists.txt @@ -1,24 +1,23 @@ # Copyright Vespa.ai. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -vespa_add_executable(searchcore_selectpruner_test_app TEST +vespa_add_executable(searchcore_proton_common_vespa_test_app TEST SOURCES + vespa_testrunner.cpp selectpruner_test.cpp - DEPENDS - searchcore_pcommon - searchlib_test -) -vespa_add_test(NAME searchcore_selectpruner_test_app COMMAND searchcore_selectpruner_test_app) -vespa_add_executable(searchcore_cachedselect_test_app TEST - SOURCES cachedselect_test.cpp - DEPENDS - searchcore_pcommon - searchlib_test -) -vespa_add_test(NAME searchcore_cachedselect_test_app COMMAND searchcore_cachedselect_test_app) -vespa_add_executable(pendinglidtracker_test_app TEST - SOURCES pendinglidtracker_test.cpp + attribute_updater_test.cpp + state_reporter_utils_test.cpp + document_type_inspector_test.cpp + feedoperation_test.cpp + documentdb_job_trackers_test.cpp + job_load_sampler_test.cpp + job_tracked_flush_test.cpp + statusreport_test.cpp DEPENDS + searchcore_proton_metrics + searchcore_feedoperation searchcore_pcommon + searchcore_test + searchlib_test ) -vespa_add_test(NAME pendinglidtracker_test_app COMMAND pendinglidtracker_test_app) +vespa_add_test(NAME searchcore_proton_common_vespa_test_app COMMAND searchcore_proton_common_vespa_test_app) diff --git a/searchcore/src/tests/proton/common/attribute_updater/.gitignore b/searchcore/src/tests/proton/common/attribute_updater/.gitignore deleted file mode 100644 index 3c6e15d6808..00000000000 --- a/searchcore/src/tests/proton/common/attribute_updater/.gitignore +++ /dev/null @@ -1,4 +0,0 @@ -.depend -Makefile -attribute_updater_test -searchcore_attribute_updater_test_app diff --git a/searchcore/src/tests/proton/common/attribute_updater/CMakeLists.txt b/searchcore/src/tests/proton/common/attribute_updater/CMakeLists.txt deleted file mode 100644 index be0da1012d0..00000000000 --- a/searchcore/src/tests/proton/common/attribute_updater/CMakeLists.txt +++ /dev/null @@ -1,9 +0,0 @@ -# Copyright Vespa.ai. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -vespa_add_executable(searchcore_attribute_updater_test_app TEST - SOURCES - attribute_updater_test.cpp - DEPENDS - searchcore_pcommon - searchlib_test -) -vespa_add_test(NAME searchcore_attribute_updater_test_app COMMAND searchcore_attribute_updater_test_app) diff --git a/searchcore/src/tests/proton/common/attribute_updater/attribute_updater_test.cpp b/searchcore/src/tests/proton/common/attribute_updater_test.cpp index b2d0abaa93a..432386af0e6 100644 --- a/searchcore/src/tests/proton/common/attribute_updater/attribute_updater_test.cpp +++ b/searchcore/src/tests/proton/common/attribute_updater_test.cpp @@ -40,9 +40,6 @@ #include <vespa/vespalib/test/insertion_operators.h> #include <vespa/vespalib/testkit/test_kit.h> -#include <vespa/log/log.h> -LOG_SETUP("attribute_updater_test"); - using namespace document; using document::config_builder::Array; using document::config_builder::Map; @@ -472,6 +469,3 @@ TEST_F("require that tensor remove update is applied", } } - -TEST_MAIN() { TEST_RUN_ALL(); } - diff --git a/searchcore/src/tests/proton/common/cachedselect_test.cpp b/searchcore/src/tests/proton/common/cachedselect_test.cpp index 5155db1534d..70cec30392b 100644 --- a/searchcore/src/tests/proton/common/cachedselect_test.cpp +++ b/searchcore/src/tests/proton/common/cachedselect_test.cpp @@ -24,7 +24,7 @@ #include <vespa/vespalib/testkit/test_kit.h> #include <vespa/log/log.h> -LOG_SETUP("cachedselect_test"); +LOG_SETUP(".cachedselect_test"); using document::DataType; using document::Document; @@ -145,9 +145,7 @@ checkSelect(const NodeUP &sel, } std::ostringstream os; EXPECT_TRUE(sel->trace(ctx, os) == exp); - LOG(info, - "trace output: '%s'", - os.str().c_str()); + LOG(info, "trace output: '%s'", os.str().c_str()); return false; } @@ -679,7 +677,4 @@ TEST_F("Test performance when using attributes", TestFixture) } - } - -TEST_MAIN() { TEST_RUN_ALL(); } diff --git a/searchcore/src/tests/proton/common/document_type_inspector/.gitignore b/searchcore/src/tests/proton/common/document_type_inspector/.gitignore deleted file mode 100644 index 49db4ae7746..00000000000 --- a/searchcore/src/tests/proton/common/document_type_inspector/.gitignore +++ /dev/null @@ -1 +0,0 @@ -searchcore_document_type_inspector_test_app diff --git a/searchcore/src/tests/proton/common/document_type_inspector/CMakeLists.txt b/searchcore/src/tests/proton/common/document_type_inspector/CMakeLists.txt deleted file mode 100644 index 339574dc906..00000000000 --- a/searchcore/src/tests/proton/common/document_type_inspector/CMakeLists.txt +++ /dev/null @@ -1,8 +0,0 @@ -# Copyright Vespa.ai. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -vespa_add_executable(searchcore_document_type_inspector_test_app TEST - SOURCES - document_type_inspector_test.cpp - DEPENDS - searchcore_pcommon -) -vespa_add_test(NAME searchcore_document_type_inspector_test_app COMMAND searchcore_document_type_inspector_test_app) diff --git a/searchcore/src/tests/proton/common/document_type_inspector/document_type_inspector_test.cpp b/searchcore/src/tests/proton/common/document_type_inspector_test.cpp index c75641ab99a..46022ed9273 100644 --- a/searchcore/src/tests/proton/common/document_type_inspector/document_type_inspector_test.cpp +++ b/searchcore/src/tests/proton/common/document_type_inspector_test.cpp @@ -127,8 +127,3 @@ TEST_F("require that struct addition is detected", Fixture(false, false)) EXPECT_FALSE(inspector.hasUnchangedField("map.key")); EXPECT_FALSE(inspector.hasUnchangedField("map.value")); } - -TEST_MAIN() -{ - TEST_RUN_ALL(); -} diff --git a/searchcore/src/tests/proton/metrics/documentdb_job_trackers/documentdb_job_trackers_test.cpp b/searchcore/src/tests/proton/common/documentdb_job_trackers_test.cpp index 721bae932f5..89c3b164e96 100644 --- a/searchcore/src/tests/proton/metrics/documentdb_job_trackers/documentdb_job_trackers_test.cpp +++ b/searchcore/src/tests/proton/common/documentdb_job_trackers_test.cpp @@ -6,9 +6,6 @@ #include <vespa/vespalib/testkit/test_kit.h> #include <thread> -#include <vespa/log/log.h> -LOG_SETUP("documentdb_job_trackers_test"); - using namespace proton; using namespace searchcorespi; @@ -115,5 +112,3 @@ TEST_F("require that un-known flush targets are not tracked", Fixture) EXPECT_EQUAL(1u, output.size()); EXPECT_EQUAL(&*output[0].get(), &*input[0]); } - -TEST_MAIN() { TEST_RUN_ALL(); } diff --git a/searchcore/src/tests/proton/feedoperation/feedoperation_test.cpp b/searchcore/src/tests/proton/common/feedoperation_test.cpp index 8d14a73626a..48893aa7da3 100644 --- a/searchcore/src/tests/proton/feedoperation/feedoperation_test.cpp +++ b/searchcore/src/tests/proton/common/feedoperation_test.cpp @@ -357,5 +357,3 @@ TEST_F("require that we can serialize and deserialize remove by gid operations", } } // namespace - -TEST_MAIN() { TEST_RUN_ALL(); } diff --git a/searchcore/src/tests/proton/metrics/job_load_sampler/job_load_sampler_test.cpp b/searchcore/src/tests/proton/common/job_load_sampler_test.cpp index f313ca43848..b6fcd3fe092 100644 --- a/searchcore/src/tests/proton/metrics/job_load_sampler/job_load_sampler_test.cpp +++ b/searchcore/src/tests/proton/common/job_load_sampler_test.cpp @@ -1,6 +1,4 @@ // Copyright Vespa.ai. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -#include <vespa/log/log.h> -LOG_SETUP("job_load_sampler_test"); #include <vespa/searchcore/proton/metrics/job_load_sampler.h> #include <vespa/vespalib/testkit/test_kit.h> @@ -101,5 +99,3 @@ TEST_F("require that multiple jobs that starts and ends in several intervals get f.end(45); EXPECT_APPROX(0.5, f.sample(50), EPS); } - -TEST_MAIN() { TEST_RUN_ALL(); } diff --git a/searchcore/src/tests/proton/metrics/job_tracked_flush/job_tracked_flush_test.cpp b/searchcore/src/tests/proton/common/job_tracked_flush_test.cpp index a10fd4eeeec..608fbb60a70 100644 --- a/searchcore/src/tests/proton/metrics/job_tracked_flush/job_tracked_flush_test.cpp +++ b/searchcore/src/tests/proton/common/job_tracked_flush_test.cpp @@ -10,9 +10,6 @@ #include <vespa/vespalib/util/threadstackexecutor.h> #include <vespa/vespalib/util/gate.h> -#include <vespa/log/log.h> -LOG_SETUP("job_tracked_flush_test"); - using namespace proton; using namespace searchcorespi; using search::SerialNum; @@ -134,5 +131,3 @@ TEST_F("require that nullptr flush task is not tracked", Fixture) FlushTask::UP task = f._trackedFlush.initFlush(0, std::make_shared<search::FlushToken>()); EXPECT_TRUE(task.get() == nullptr); } - -TEST_MAIN() { TEST_RUN_ALL(); } diff --git a/searchcore/src/tests/proton/common/pendinglidtracker_test.cpp b/searchcore/src/tests/proton/common/pendinglidtracker_test.cpp index 29d7dbfeec1..f8d0d218670 100644 --- a/searchcore/src/tests/proton/common/pendinglidtracker_test.cpp +++ b/searchcore/src/tests/proton/common/pendinglidtracker_test.cpp @@ -3,9 +3,6 @@ #include <vespa/vespalib/testkit/test_kit.h> #include <vespa/searchcore/proton/common/pendinglidtracker.h> -#include <vespa/log/log.h> -LOG_SETUP("pendinglidtracker_test"); - using namespace proton; constexpr uint32_t LID_1 = 1u; @@ -76,5 +73,3 @@ TEST("test pendinglidtracker for needcommit") { EXPECT_EQUAL(ILidCommitState::State::COMPLETED, tracker.getState(LID_1)); EXPECT_EQUAL(ILidCommitState::State::COMPLETED, tracker.getState(LIDV_2_1_3)); } - -TEST_MAIN() { TEST_RUN_ALL(); } diff --git a/searchcore/src/tests/proton/common/selectpruner_test.cpp b/searchcore/src/tests/proton/common/selectpruner_test.cpp index d309fe73715..09854af99e1 100644 --- a/searchcore/src/tests/proton/common/selectpruner_test.cpp +++ b/searchcore/src/tests/proton/common/selectpruner_test.cpp @@ -13,7 +13,7 @@ #include <vespa/vespalib/testkit/test_kit.h> #include <vespa/log/log.h> -LOG_SETUP("selectpruner_test"); +LOG_SETUP(".selectpruner_test"); using document::DataType; using document::Document; @@ -824,5 +824,3 @@ TEST_F("Complex imported field references return Invalid", TestFixture) } } // namespace - -TEST_MAIN() { TEST_RUN_ALL(); } diff --git a/searchcore/src/tests/proton/common/state_reporter_utils/.gitignore b/searchcore/src/tests/proton/common/state_reporter_utils/.gitignore deleted file mode 100644 index bb0963e5ec3..00000000000 --- a/searchcore/src/tests/proton/common/state_reporter_utils/.gitignore +++ /dev/null @@ -1 +0,0 @@ -searchcore_state_reporter_utils_test_app diff --git a/searchcore/src/tests/proton/common/state_reporter_utils/CMakeLists.txt b/searchcore/src/tests/proton/common/state_reporter_utils/CMakeLists.txt deleted file mode 100644 index 1bdb0b613cf..00000000000 --- a/searchcore/src/tests/proton/common/state_reporter_utils/CMakeLists.txt +++ /dev/null @@ -1,8 +0,0 @@ -# Copyright Vespa.ai. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -vespa_add_executable(searchcore_state_reporter_utils_test_app TEST - SOURCES - state_reporter_utils_test.cpp - DEPENDS - searchcore_pcommon -) -vespa_add_test(NAME searchcore_state_reporter_utils_test_app COMMAND searchcore_state_reporter_utils_test_app) diff --git a/searchcore/src/tests/proton/common/state_reporter_utils/state_reporter_utils_test.cpp b/searchcore/src/tests/proton/common/state_reporter_utils_test.cpp index 513df454e94..6c9025d276f 100644 --- a/searchcore/src/tests/proton/common/state_reporter_utils/state_reporter_utils_test.cpp +++ b/searchcore/src/tests/proton/common/state_reporter_utils_test.cpp @@ -1,6 +1,4 @@ // Copyright Vespa.ai. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -#include <vespa/log/log.h> -LOG_SETUP("state_reporter_utils_test"); #include <vespa/searchcore/proton/common/state_reporter_utils.h> #include <vespa/vespalib/data/slime/slime.h> @@ -44,4 +42,3 @@ TEST("require that advanced status report is correctly converted to slime") message("foo")))); } -TEST_MAIN() { TEST_RUN_ALL(); } diff --git a/searchcore/src/tests/proton/statusreport/statusreport_test.cpp b/searchcore/src/tests/proton/common/statusreport_test.cpp index a4d9687fa07..052eb795529 100644 --- a/searchcore/src/tests/proton/statusreport/statusreport_test.cpp +++ b/searchcore/src/tests/proton/common/statusreport_test.cpp @@ -37,5 +37,3 @@ TEST("require that custom status report works") } } // namespace proton - -TEST_MAIN() { TEST_RUN_ALL(); } diff --git a/searchcore/src/tests/proton/common/vespa_testrunner.cpp b/searchcore/src/tests/proton/common/vespa_testrunner.cpp new file mode 100644 index 00000000000..1e4e79047c3 --- /dev/null +++ b/searchcore/src/tests/proton/common/vespa_testrunner.cpp @@ -0,0 +1,8 @@ +// Copyright Vespa.ai. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +// Unit tests for predicate_index. +#include <vespa/vespalib/testkit/test_kit.h> + +#include <vespa/log/log.h> +LOG_SETUP("proton_common_test"); + +TEST_MAIN() { TEST_RUN_ALL(); } diff --git a/searchcore/src/tests/proton/feedoperation/.gitignore b/searchcore/src/tests/proton/feedoperation/.gitignore deleted file mode 100644 index cfdeb9049b2..00000000000 --- a/searchcore/src/tests/proton/feedoperation/.gitignore +++ /dev/null @@ -1,4 +0,0 @@ -*_test -.depend -Makefile -searchcore_feedoperation_test_app diff --git a/searchcore/src/tests/proton/feedoperation/CMakeLists.txt b/searchcore/src/tests/proton/feedoperation/CMakeLists.txt deleted file mode 100644 index fe9d3bad302..00000000000 --- a/searchcore/src/tests/proton/feedoperation/CMakeLists.txt +++ /dev/null @@ -1,9 +0,0 @@ -# Copyright Vespa.ai. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -vespa_add_executable(searchcore_feedoperation_test_app TEST - SOURCES - feedoperation_test.cpp - DEPENDS - searchcore_feedoperation - searchcore_pcommon -) -vespa_add_test(NAME searchcore_feedoperation_test_app COMMAND searchcore_feedoperation_test_app) diff --git a/searchcore/src/tests/proton/metrics/documentdb_job_trackers/.gitignore b/searchcore/src/tests/proton/metrics/documentdb_job_trackers/.gitignore deleted file mode 100644 index 84c97c63aca..00000000000 --- a/searchcore/src/tests/proton/metrics/documentdb_job_trackers/.gitignore +++ /dev/null @@ -1 +0,0 @@ -searchcore_documentdb_job_trackers_test_app diff --git a/searchcore/src/tests/proton/metrics/documentdb_job_trackers/CMakeLists.txt b/searchcore/src/tests/proton/metrics/documentdb_job_trackers/CMakeLists.txt deleted file mode 100644 index 81d054e2242..00000000000 --- a/searchcore/src/tests/proton/metrics/documentdb_job_trackers/CMakeLists.txt +++ /dev/null @@ -1,9 +0,0 @@ -# Copyright Vespa.ai. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -vespa_add_executable(searchcore_documentdb_job_trackers_test_app TEST - SOURCES - documentdb_job_trackers_test.cpp - DEPENDS - searchcore_proton_metrics - searchcore_test -) -vespa_add_test(NAME searchcore_documentdb_job_trackers_test_app COMMAND searchcore_documentdb_job_trackers_test_app) diff --git a/searchcore/src/tests/proton/metrics/job_load_sampler/.gitignore b/searchcore/src/tests/proton/metrics/job_load_sampler/.gitignore deleted file mode 100644 index 2e02ec8191b..00000000000 --- a/searchcore/src/tests/proton/metrics/job_load_sampler/.gitignore +++ /dev/null @@ -1 +0,0 @@ -searchcore_job_load_sampler_test_app diff --git a/searchcore/src/tests/proton/metrics/job_load_sampler/CMakeLists.txt b/searchcore/src/tests/proton/metrics/job_load_sampler/CMakeLists.txt deleted file mode 100644 index 955b1e14028..00000000000 --- a/searchcore/src/tests/proton/metrics/job_load_sampler/CMakeLists.txt +++ /dev/null @@ -1,8 +0,0 @@ -# Copyright Vespa.ai. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -vespa_add_executable(searchcore_job_load_sampler_test_app TEST - SOURCES - job_load_sampler_test.cpp - DEPENDS - searchcore_proton_metrics -) -vespa_add_test(NAME searchcore_job_load_sampler_test_app COMMAND searchcore_job_load_sampler_test_app) diff --git a/searchcore/src/tests/proton/metrics/job_tracked_flush/.gitignore b/searchcore/src/tests/proton/metrics/job_tracked_flush/.gitignore deleted file mode 100644 index 85e6097878b..00000000000 --- a/searchcore/src/tests/proton/metrics/job_tracked_flush/.gitignore +++ /dev/null @@ -1 +0,0 @@ -searchcore_job_tracked_flush_test_app diff --git a/searchcore/src/tests/proton/metrics/job_tracked_flush/CMakeLists.txt b/searchcore/src/tests/proton/metrics/job_tracked_flush/CMakeLists.txt deleted file mode 100644 index a4f0ff0bcec..00000000000 --- a/searchcore/src/tests/proton/metrics/job_tracked_flush/CMakeLists.txt +++ /dev/null @@ -1,9 +0,0 @@ -# Copyright Vespa.ai. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -vespa_add_executable(searchcore_job_tracked_flush_test_app TEST - SOURCES - job_tracked_flush_test.cpp - DEPENDS - searchcore_proton_metrics - searchcore_test -) -vespa_add_test(NAME searchcore_job_tracked_flush_test_app COMMAND searchcore_job_tracked_flush_test_app) diff --git a/searchcore/src/tests/proton/statusreport/.gitignore b/searchcore/src/tests/proton/statusreport/.gitignore deleted file mode 100644 index 68753df292a..00000000000 --- a/searchcore/src/tests/proton/statusreport/.gitignore +++ /dev/null @@ -1 +0,0 @@ -searchcore_statusreport_test_app diff --git a/searchcore/src/tests/proton/statusreport/CMakeLists.txt b/searchcore/src/tests/proton/statusreport/CMakeLists.txt deleted file mode 100644 index 1f2a3cfbec1..00000000000 --- a/searchcore/src/tests/proton/statusreport/CMakeLists.txt +++ /dev/null @@ -1,8 +0,0 @@ -# Copyright Vespa.ai. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -vespa_add_executable(searchcore_statusreport_test_app TEST - SOURCES - statusreport_test.cpp - DEPENDS - searchcore_pcommon -) -vespa_add_test(NAME searchcore_statusreport_test_app COMMAND searchcore_statusreport_test_app) diff --git a/searchcore/src/vespa/searchcore/grouping/groupingcontext.cpp b/searchcore/src/vespa/searchcore/grouping/groupingcontext.cpp index e21cb4c1282..88939958aa0 100644 --- a/searchcore/src/vespa/searchcore/grouping/groupingcontext.cpp +++ b/searchcore/src/vespa/searchcore/grouping/groupingcontext.cpp @@ -53,11 +53,7 @@ GroupingContext::setDistributionKey(uint32_t distributionKey) GroupingContext::GroupingContext(const BitVector & validLids, const std::atomic<steady_time> & now_ref, vespalib::steady_time timeOfDoom, const char *groupSpec, uint32_t groupSpecLen) - : _validLids(validLids), - _now_ref(now_ref), - _timeOfDoom(timeOfDoom), - _os(), - _groupingList() + : GroupingContext(validLids, now_ref, timeOfDoom) { deserialize(groupSpec, groupSpecLen); } @@ -71,11 +67,7 @@ GroupingContext::GroupingContext(const BitVector & validLids, const std::atomic< { } GroupingContext::GroupingContext(const GroupingContext & rhs) - : _validLids(rhs._validLids), - _now_ref(rhs._now_ref), - _timeOfDoom(rhs._timeOfDoom), - _os(), - _groupingList() + : GroupingContext(rhs._validLids, rhs._now_ref, rhs._timeOfDoom) { } void diff --git a/searchcore/src/vespa/searchcore/grouping/groupingsession.cpp b/searchcore/src/vespa/searchcore/grouping/groupingsession.cpp index ef45b36b551..e3f8919e747 100644 --- a/searchcore/src/vespa/searchcore/grouping/groupingsession.cpp +++ b/searchcore/src/vespa/searchcore/grouping/groupingsession.cpp @@ -27,18 +27,11 @@ GroupingSession::GroupingSession(const SessionId &sessionId, GroupingSession::~GroupingSession() = default; -using search::expression::ExpressionNode; -using search::expression::AttributeNode; -using search::expression::ConfigureStaticParams; -using search::aggregation::Grouping; -using search::aggregation::GroupingLevel; - void GroupingSession::init(GroupingContext & groupingContext, const IAttributeContext &attrCtx) { GroupingList & sessionList(groupingContext.getGroupingList()); - for (size_t i = 0; i < sessionList.size(); ++i) { - GroupingPtr g(sessionList[i]); + for (auto g : sessionList) { // Make internal copy of those we want to keep for another pass if (!_sessionId.empty() && g->getLastLevel() < g->levels().size()) { auto gp = std::make_shared<Grouping>(*g); diff --git a/searchlib/src/tests/bitcompression/expgolomb/expgolomb_test.cpp b/searchlib/src/tests/bitcompression/expgolomb/expgolomb_test.cpp index 6857ca93e6e..f76476dfda2 100644 --- a/searchlib/src/tests/bitcompression/expgolomb/expgolomb_test.cpp +++ b/searchlib/src/tests/bitcompression/expgolomb/expgolomb_test.cpp @@ -6,6 +6,7 @@ #include <vector> #include <algorithm> #include <cinttypes> +#include <cassert> #include <vespa/log/log.h> LOG_SETUP("expglomb_test"); diff --git a/searchlib/src/tests/predicate/.gitignore b/searchlib/src/tests/predicate/.gitignore index eea4d347d05..56e1330505c 100644 --- a/searchlib/src/tests/predicate/.gitignore +++ b/searchlib/src/tests/predicate/.gitignore @@ -1,13 +1 @@ -searchlib_document_features_store_test_app -searchlib_predicate_bounds_posting_list_test_app -searchlib_predicate_index_test_app -searchlib_predicate_interval_posting_list_test_app -searchlib_predicate_interval_store_test_app -searchlib_predicate_range_term_expander_test_app -searchlib_predicate_ref_cache_test_app -searchlib_predicate_tree_analyzer_test_app -searchlib_predicate_tree_annotator_test_app -searchlib_predicate_zero_constraint_posting_list_test_app -searchlib_predicate_zstar_compressed_posting_list_test_app -searchlib_simple_index_test_app -searchlib_tree_crumbs_test_app +searchlib_predicate_vespa_test_app diff --git a/searchlib/src/tests/predicate/CMakeLists.txt b/searchlib/src/tests/predicate/CMakeLists.txt index d33e5617908..4ae06af1886 100644 --- a/searchlib/src/tests/predicate/CMakeLists.txt +++ b/searchlib/src/tests/predicate/CMakeLists.txt @@ -1,92 +1,21 @@ # Copyright Vespa.ai. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -vespa_add_executable(searchlib_predicate_index_test_app TEST +vespa_add_executable(searchlib_predicate_vespa_test_app TEST SOURCES + vespa_testrunner.cpp predicate_index_test.cpp - DEPENDS - vespa_searchlib -) -vespa_add_test(NAME searchlib_predicate_index_test_app COMMAND searchlib_predicate_index_test_app) -vespa_add_executable(searchlib_simple_index_test_app TEST - SOURCES simple_index_test.cpp - DEPENDS - vespa_searchlib -) -vespa_add_test(NAME searchlib_simple_index_test_app COMMAND searchlib_simple_index_test_app) -vespa_add_executable(searchlib_tree_crumbs_test_app TEST - SOURCES tree_crumbs_test.cpp - DEPENDS - vespa_searchlib -) -vespa_add_test(NAME searchlib_tree_crumbs_test_app COMMAND searchlib_tree_crumbs_test_app) -vespa_add_executable(searchlib_predicate_tree_analyzer_test_app TEST - SOURCES predicate_tree_analyzer_test.cpp - DEPENDS - vespa_searchlib -) -vespa_add_test(NAME searchlib_predicate_tree_analyzer_test_app COMMAND searchlib_predicate_tree_analyzer_test_app) -vespa_add_executable(searchlib_predicate_tree_annotator_test_app TEST - SOURCES predicate_tree_annotator_test.cpp - DEPENDS - vespa_searchlib -) -vespa_add_test(NAME searchlib_predicate_tree_annotator_test_app COMMAND searchlib_predicate_tree_annotator_test_app) -vespa_add_executable(searchlib_predicate_interval_store_test_app TEST - SOURCES predicate_interval_store_test.cpp - DEPENDS - vespa_searchlib -) -vespa_add_test(NAME searchlib_predicate_interval_store_test_app COMMAND searchlib_predicate_interval_store_test_app) -vespa_add_executable(searchlib_document_features_store_test_app TEST - SOURCES document_features_store_test.cpp - DEPENDS - vespa_searchlib -) -vespa_add_test(NAME searchlib_document_features_store_test_app COMMAND searchlib_document_features_store_test_app) -vespa_add_executable(searchlib_predicate_ref_cache_test_app TEST - SOURCES predicate_ref_cache_test.cpp - DEPENDS - vespa_searchlib -) -vespa_add_test(NAME searchlib_predicate_ref_cache_test_app COMMAND searchlib_predicate_ref_cache_test_app) -vespa_add_executable(searchlib_predicate_interval_posting_list_test_app TEST - SOURCES predicate_interval_posting_list_test.cpp - DEPENDS - vespa_searchlib -) -vespa_add_test(NAME searchlib_predicate_interval_posting_list_test_app COMMAND searchlib_predicate_interval_posting_list_test_app) -vespa_add_executable(searchlib_predicate_bounds_posting_list_test_app TEST - SOURCES predicate_bounds_posting_list_test.cpp - DEPENDS - vespa_searchlib -) -vespa_add_test(NAME searchlib_predicate_bounds_posting_list_test_app COMMAND searchlib_predicate_bounds_posting_list_test_app) -vespa_add_executable(searchlib_predicate_zero_constraint_posting_list_test_app TEST - SOURCES predicate_zero_constraint_posting_list_test.cpp - DEPENDS - vespa_searchlib -) -vespa_add_test(NAME searchlib_predicate_zero_constraint_posting_list_test_app COMMAND searchlib_predicate_zero_constraint_posting_list_test_app) -vespa_add_executable(searchlib_predicate_zstar_compressed_posting_list_test_app TEST - SOURCES predicate_zstar_compressed_posting_list_test.cpp - DEPENDS - vespa_searchlib -) -vespa_add_test(NAME searchlib_predicate_zstar_compressed_posting_list_test_app COMMAND searchlib_predicate_zstar_compressed_posting_list_test_app) -vespa_add_executable(searchlib_predicate_range_term_expander_test_app TEST - SOURCES predicate_range_term_expander_test.cpp DEPENDS vespa_searchlib ) -vespa_add_test(NAME searchlib_predicate_range_term_expander_test_app COMMAND searchlib_predicate_range_term_expander_test_app) +vespa_add_test(NAME searchlib_predicate_vespa_test_app COMMAND searchlib_predicate_vespa_test_app) diff --git a/searchlib/src/tests/predicate/document_features_store_test.cpp b/searchlib/src/tests/predicate/document_features_store_test.cpp index cc090a953b5..01eaa75a71a 100644 --- a/searchlib/src/tests/predicate/document_features_store_test.cpp +++ b/searchlib/src/tests/predicate/document_features_store_test.cpp @@ -8,10 +8,6 @@ #include <vespa/searchlib/predicate/predicate_tree_annotator.h> #include <vespa/searchlib/predicate/predicate_hash.h> #include <vespa/vespalib/testkit/test_kit.h> -#include <string> - -#include <vespa/log/log.h> -LOG_SETUP("document_features_store_test"); using namespace search; using namespace search::predicate; @@ -233,5 +229,3 @@ TEST("require that serialization cleans up wordstore") { } // namespace - -TEST_MAIN() { TEST_RUN_ALL(); } diff --git a/searchlib/src/tests/predicate/predicate_bounds_posting_list_test.cpp b/searchlib/src/tests/predicate/predicate_bounds_posting_list_test.cpp index 91e9fc0f9a4..be7f4516bb2 100644 --- a/searchlib/src/tests/predicate/predicate_bounds_posting_list_test.cpp +++ b/searchlib/src/tests/predicate/predicate_bounds_posting_list_test.cpp @@ -9,9 +9,6 @@ #include <vespa/vespalib/btree/btreestore.hpp> #include <vespa/vespalib/testkit/test_kit.h> -#include <vespa/log/log.h> -LOG_SETUP("predicate_bounds_posting_list_test"); - using namespace search; using namespace search::predicate; @@ -106,5 +103,3 @@ TEST("require that bounds posting list checks bounds.") { } } // namespace - -TEST_MAIN() { TEST_RUN_ALL(); } diff --git a/searchlib/src/tests/predicate/predicate_index_test.cpp b/searchlib/src/tests/predicate/predicate_index_test.cpp index 120862a1477..d0af12f93c7 100644 --- a/searchlib/src/tests/predicate/predicate_index_test.cpp +++ b/searchlib/src/tests/predicate/predicate_index_test.cpp @@ -12,9 +12,6 @@ #include <vespa/vespalib/btree/btreeiterator.hpp> #include <vespa/vespalib/btree/btreestore.hpp> -#include <vespa/log/log.h> -LOG_SETUP("predicate_index_test"); - using namespace search; using namespace search::predicate; using std::make_pair; @@ -454,5 +451,3 @@ TEST("require that predicate index saver protected by a generation guard observe } } // namespace - -TEST_MAIN() { TEST_RUN_ALL(); } diff --git a/searchlib/src/tests/predicate/predicate_interval_posting_list_test.cpp b/searchlib/src/tests/predicate/predicate_interval_posting_list_test.cpp index 3977933dbc3..098c0238412 100644 --- a/searchlib/src/tests/predicate/predicate_interval_posting_list_test.cpp +++ b/searchlib/src/tests/predicate/predicate_interval_posting_list_test.cpp @@ -9,9 +9,6 @@ #include <vespa/vespalib/btree/btreestore.hpp> #include <vespa/vespalib/testkit/test_kit.h> -#include <vespa/log/log.h> -LOG_SETUP("predicate_interval_posting_list_test"); - using namespace search; using namespace search::predicate; namespace { @@ -79,5 +76,3 @@ TEST("require that posting list can iterate.") { } } // namespace - -TEST_MAIN() { TEST_RUN_ALL(); } diff --git a/searchlib/src/tests/predicate/predicate_interval_store_test.cpp b/searchlib/src/tests/predicate/predicate_interval_store_test.cpp index 9c2847bc8cf..d8c9691d421 100644 --- a/searchlib/src/tests/predicate/predicate_interval_store_test.cpp +++ b/searchlib/src/tests/predicate/predicate_interval_store_test.cpp @@ -1,9 +1,6 @@ // Copyright Vespa.ai. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. // Unit tests for predicate_interval_store. -#include <vespa/log/log.h> -LOG_SETUP("predicate_interval_store_test"); - #include <vespa/searchlib/predicate/predicate_interval_store.h> #include <vespa/searchlib/predicate/predicate_index.h> @@ -147,5 +144,3 @@ TEST("require that interval refs are reused for identical data.") { } } // namespace - -TEST_MAIN() { TEST_RUN_ALL(); } diff --git a/searchlib/src/tests/predicate/predicate_range_term_expander_test.cpp b/searchlib/src/tests/predicate/predicate_range_term_expander_test.cpp index bec1cd9f24a..2dce3c50b0a 100644 --- a/searchlib/src/tests/predicate/predicate_range_term_expander_test.cpp +++ b/searchlib/src/tests/predicate/predicate_range_term_expander_test.cpp @@ -1,9 +1,6 @@ // Copyright Vespa.ai. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. // Unit tests for predicate_range_term_expander. -#include <vespa/log/log.h> -LOG_SETUP("predicate_range_term_expander_test"); - #include <vespa/searchlib/predicate/predicate_range_term_expander.h> #include <vespa/vespalib/btree/btreestore.hpp> #include <vespa/vespalib/testkit/test_kit.h> @@ -328,5 +325,3 @@ TEST("require that search close to max uneven upper bound is sensible") { } } // namespace - -TEST_MAIN() { TEST_RUN_ALL(); } diff --git a/searchlib/src/tests/predicate/predicate_ref_cache_test.cpp b/searchlib/src/tests/predicate/predicate_ref_cache_test.cpp index 692482b8933..f62f3f807c5 100644 --- a/searchlib/src/tests/predicate/predicate_ref_cache_test.cpp +++ b/searchlib/src/tests/predicate/predicate_ref_cache_test.cpp @@ -1,9 +1,6 @@ // Copyright Vespa.ai. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. // Unit tests for predicate_ref_cache. -#include <vespa/log/log.h> -LOG_SETUP("predicate_ref_cache_test"); - #include <vespa/searchlib/predicate/predicate_ref_cache.h> #include <vespa/vespalib/testkit/test_kit.h> #include <vector> @@ -101,5 +98,3 @@ TEST("require that cache handles large entries") { } } // namespace - -TEST_MAIN() { TEST_RUN_ALL(); } diff --git a/searchlib/src/tests/predicate/predicate_tree_analyzer_test.cpp b/searchlib/src/tests/predicate/predicate_tree_analyzer_test.cpp index 854abcd6fda..73a236aa443 100644 --- a/searchlib/src/tests/predicate/predicate_tree_analyzer_test.cpp +++ b/searchlib/src/tests/predicate/predicate_tree_analyzer_test.cpp @@ -1,9 +1,6 @@ // Copyright Vespa.ai. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. // Unit tests for PredicateTreeAnalyzer. -#include <vespa/log/log.h> -LOG_SETUP("PredicateTreeAnalyzer_test"); - #include <vespa/document/predicate/predicate.h> #include <vespa/document/predicate/predicate_slime_builder.h> #include <vespa/searchlib/predicate/predicate_tree_analyzer.h> @@ -152,5 +149,3 @@ TEST("require that multilevel AND stores sizes") { } } // namespace - -TEST_MAIN() { TEST_RUN_ALL(); } diff --git a/searchlib/src/tests/predicate/predicate_tree_annotator_test.cpp b/searchlib/src/tests/predicate/predicate_tree_annotator_test.cpp index fa586153f19..4d71f585910 100644 --- a/searchlib/src/tests/predicate/predicate_tree_annotator_test.cpp +++ b/searchlib/src/tests/predicate/predicate_tree_annotator_test.cpp @@ -10,9 +10,6 @@ #include <vespa/vespalib/testkit/test_kit.h> #include <sstream> -#include <vespa/log/log.h> -LOG_SETUP("PredicateTreeAnnotator_test"); - using document::Predicate; using std::ostringstream; using std::pair; @@ -403,5 +400,3 @@ TEST("require that open range works") { } } // namespace - -TEST_MAIN() { TEST_RUN_ALL(); } diff --git a/searchlib/src/tests/predicate/predicate_zero_constraint_posting_list_test.cpp b/searchlib/src/tests/predicate/predicate_zero_constraint_posting_list_test.cpp index 83d29102a7b..5907dce72ba 100644 --- a/searchlib/src/tests/predicate/predicate_zero_constraint_posting_list_test.cpp +++ b/searchlib/src/tests/predicate/predicate_zero_constraint_posting_list_test.cpp @@ -6,9 +6,6 @@ #include <vespa/vespalib/testkit/test_kit.h> -#include <vespa/log/log.h> -LOG_SETUP("predicate_zero_constraint_posting_list_test"); - using namespace search; using namespace search::predicate; @@ -54,5 +51,3 @@ TEST("require that posting list can iterate.") { } } // namespace - -TEST_MAIN() { TEST_RUN_ALL(); } diff --git a/searchlib/src/tests/predicate/predicate_zstar_compressed_posting_list_test.cpp b/searchlib/src/tests/predicate/predicate_zstar_compressed_posting_list_test.cpp index a7011ec51d2..20cd0809473 100644 --- a/searchlib/src/tests/predicate/predicate_zstar_compressed_posting_list_test.cpp +++ b/searchlib/src/tests/predicate/predicate_zstar_compressed_posting_list_test.cpp @@ -8,9 +8,6 @@ #include <vespa/vespalib/btree/btreestore.hpp> #include <vespa/vespalib/testkit/test_kit.h> -#include <vespa/log/log.h> -LOG_SETUP("predicate_zstar_compressed_posting_list_test"); - using namespace search; using namespace search::predicate; using std::vector; @@ -93,5 +90,3 @@ TEST("require that posting list can iterate.") { } } // namespace - -TEST_MAIN() { TEST_RUN_ALL(); } diff --git a/searchlib/src/tests/predicate/simple_index_test.cpp b/searchlib/src/tests/predicate/simple_index_test.cpp index a9e5e7c9065..964ff67bd3a 100644 --- a/searchlib/src/tests/predicate/simple_index_test.cpp +++ b/searchlib/src/tests/predicate/simple_index_test.cpp @@ -16,9 +16,6 @@ #include <vespa/vespalib/util/rcuvector.hpp> #include <map> -#include <vespa/log/log.h> -LOG_SETUP("simple_index_test"); - using namespace search; using namespace search::predicate; using vespalib::GenerationHolder; @@ -342,5 +339,3 @@ TEST_F("require that vector contains correct postings", Fixture) { } } // namespace - -TEST_MAIN() { TEST_RUN_ALL(); } diff --git a/searchlib/src/tests/predicate/tree_crumbs_test.cpp b/searchlib/src/tests/predicate/tree_crumbs_test.cpp index 5c5775e5a5b..76bfd02ee50 100644 --- a/searchlib/src/tests/predicate/tree_crumbs_test.cpp +++ b/searchlib/src/tests/predicate/tree_crumbs_test.cpp @@ -1,9 +1,6 @@ // Copyright Vespa.ai. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. // Unit tests for TreeCrumbs. -#include <vespa/log/log.h> -LOG_SETUP("TreeCrumbs_test"); - #include <vespa/searchlib/predicate/tree_crumbs.h> #include <vespa/vespalib/testkit/test_kit.h> @@ -60,5 +57,3 @@ TEST("require that crumbs can set custom initial char") { } } // namespace - -TEST_MAIN() { TEST_RUN_ALL(); } diff --git a/searchlib/src/tests/predicate/vespa_testrunner.cpp b/searchlib/src/tests/predicate/vespa_testrunner.cpp new file mode 100644 index 00000000000..d812605710e --- /dev/null +++ b/searchlib/src/tests/predicate/vespa_testrunner.cpp @@ -0,0 +1,8 @@ +// Copyright Vespa.ai. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +// Unit tests for predicate_index. +#include <vespa/vespalib/testkit/test_kit.h> + +#include <vespa/log/log.h> +LOG_SETUP("predicate_test"); + +TEST_MAIN() { TEST_RUN_ALL(); } diff --git a/searchlib/src/tests/util/bufferwriter/bufferwriter_test.cpp b/searchlib/src/tests/util/bufferwriter/bufferwriter_test.cpp index d1ce5f09cfa..8faec949d86 100644 --- a/searchlib/src/tests/util/bufferwriter/bufferwriter_test.cpp +++ b/searchlib/src/tests/util/bufferwriter/bufferwriter_test.cpp @@ -5,6 +5,7 @@ #include <vespa/searchlib/util/bufferwriter.h> #include <vespa/searchlib/util/drainingbufferwriter.h> #include <vespa/vespalib/util/rand48.h> +#include <cassert> namespace search { @@ -20,7 +21,7 @@ public: static constexpr size_t BUFFER_SIZE = 262144; StoreBufferWriter(); - ~StoreBufferWriter(); + ~StoreBufferWriter() override; void flush() override; size_t getBytesWritten() const { return _bytesWritten; } diff --git a/searchlib/src/vespa/searchlib/aggregation/grouping.h b/searchlib/src/vespa/searchlib/aggregation/grouping.h index f64aa2f5a3d..77d5655e5d2 100644 --- a/searchlib/src/vespa/searchlib/aggregation/grouping.h +++ b/searchlib/src/vespa/searchlib/aggregation/grouping.h @@ -73,10 +73,8 @@ public: void postMerge(); void preAggregate(bool isOrdered); void prune(const Grouping & b); - void aggregate(DocId from, DocId to); - void aggregate(DocId docId, HitRank rank = 0); - void aggregate(const document::Document & doc, HitRank rank = 0); - void aggregate(const RankedHit * rankedHit, unsigned int len); + void aggregate(DocId docId, HitRank rank); + void aggregate(const document::Document & doc, HitRank rank); void convertToGlobalId(const IDocumentMetaStore &metaStore); void postAggregate(); void postProcess(); @@ -84,6 +82,9 @@ public: void cleanTemporary(); void configureStaticStuff(const expression::ConfigureStaticParams & params); void cleanupAttributeReferences(); + // Only used by tests + void aggregate(DocId from, DocId to); + void aggregate(const RankedHit * rankedHit, unsigned int len); }; } diff --git a/searchlib/src/vespa/searchlib/aggregation/modifiers.cpp b/searchlib/src/vespa/searchlib/aggregation/modifiers.cpp index 81664ce66eb..0749f19fceb 100644 --- a/searchlib/src/vespa/searchlib/aggregation/modifiers.cpp +++ b/searchlib/src/vespa/searchlib/aggregation/modifiers.cpp @@ -4,7 +4,6 @@ #include "grouping.h" #include <vespa/searchlib/expression/multiargfunctionnode.h> #include <vespa/searchlib/expression/attributenode.h> -#include <vespa/searchlib/expression/attribute_map_lookup_node.h> #include <vespa/searchlib/expression/documentfieldnode.h> #include <vespa/searchlib/expression/interpolated_document_field_lookup_node.h> #include <vespa/searchlib/expression/interpolatedlookupfunctionnode.h> @@ -20,44 +19,32 @@ AttributeNodeReplacer::check(const vespalib::Identifiable &obj) const } void +AttributeNodeReplacer::replaceRecurse(ExpressionNode * exp, std::function<void(ExpressionNodeUP)> && modifier) { + if (exp == nullptr) return; + if (exp->inherits(AttributeNode::classId)) { + auto replacementNode = getReplacementNode(static_cast<const AttributeNode &>(*exp)); + if (replacementNode) { + modifier(std::move(replacementNode)); + } + } else { + exp->select(*this, *this); + } +} + +void AttributeNodeReplacer::execute(vespalib::Identifiable &obj) { if (obj.getClass().inherits(GroupingLevel::classId)) { - GroupingLevel & g(static_cast<GroupingLevel &>(obj)); - if (g.getExpression().getRoot()->inherits(AttributeNode::classId)) { - auto replacementNode = getReplacementNode(static_cast<const AttributeNode &>(*g.getExpression().getRoot())); - if (replacementNode) { - g.setExpression(std::move(replacementNode)); - } - } else { - g.getExpression().getRoot()->select(*this, *this); - } + auto & g(static_cast<GroupingLevel &>(obj)); + replaceRecurse(g.getExpression().getRoot(), [&g](ExpressionNodeUP replacement) { g.setExpression(std::move(replacement)); }); g.groupPrototype().select(*this, *this); } else if(obj.getClass().inherits(AggregationResult::classId)) { - AggregationResult & a(static_cast<AggregationResult &>(obj)); - ExpressionNode * e(a.getExpression()); - if (e) { - if (e->inherits(AttributeNode::classId)) { - auto replacementNode = getReplacementNode(static_cast<const AttributeNode &>(*e)); - if (replacementNode) { - a.setExpression(std::move(replacementNode)); - } - } else { - e->select(*this, *this); - } - } + auto & a(static_cast<AggregationResult &>(obj)); + replaceRecurse(a.getExpression(), [&a](ExpressionNodeUP replacement) { a.setExpression(std::move(replacement)); }); } else if(obj.getClass().inherits(MultiArgFunctionNode::classId)) { MultiArgFunctionNode::ExpressionNodeVector & v(static_cast<MultiArgFunctionNode &>(obj).expressionNodeVector()); - for(size_t i(0), m(v.size()); i < m; i++) { - ExpressionNode::CP & e(v[i]); - if (e->inherits(AttributeNode::classId)) { - auto replacementNode = getReplacementNode(static_cast<const AttributeNode &>(*e)); - if (replacementNode) { - e = std::move(replacementNode); - } - } else { - e->select(*this, *this); - } + for (auto & e : v) { + replaceRecurse(e.get(), [&e](ExpressionNodeUP replacement) noexcept { e = std::move(replacement); }); } } } diff --git a/searchlib/src/vespa/searchlib/aggregation/modifiers.h b/searchlib/src/vespa/searchlib/aggregation/modifiers.h index 934e7111ced..e55ca28bd62 100644 --- a/searchlib/src/vespa/searchlib/aggregation/modifiers.h +++ b/searchlib/src/vespa/searchlib/aggregation/modifiers.h @@ -4,6 +4,7 @@ #include <vespa/vespalib/objects/objectoperation.h> #include <vespa/vespalib/objects/objectpredicate.h> #include <memory> +#include <functional> namespace search::expression { @@ -16,16 +17,19 @@ namespace search::aggregation { class AttributeNodeReplacer : public vespalib::ObjectOperation, public vespalib::ObjectPredicate { +protected: + using ExpressionNodeUP = std::unique_ptr<expression::ExpressionNode>; private: + void replaceRecurse(expression::ExpressionNode * exp, std::function<void(ExpressionNodeUP)> && modifier); void execute(vespalib::Identifiable &obj) override; bool check(const vespalib::Identifiable &obj) const override; - virtual std::unique_ptr<search::expression::ExpressionNode> getReplacementNode(const search::expression::AttributeNode &attributeNode) = 0; + virtual ExpressionNodeUP getReplacementNode(const expression::AttributeNode &attributeNode) = 0; }; class Attribute2DocumentAccessor : public AttributeNodeReplacer { private: - std::unique_ptr<search::expression::ExpressionNode> getReplacementNode(const search::expression::AttributeNode &attributeNode) override; + ExpressionNodeUP getReplacementNode(const expression::AttributeNode &attributeNode) override; }; } diff --git a/searchlib/src/vespa/searchlib/features/CMakeLists.txt b/searchlib/src/vespa/searchlib/features/CMakeLists.txt index 27c2b6d5e41..b468d653032 100644 --- a/searchlib/src/vespa/searchlib/features/CMakeLists.txt +++ b/searchlib/src/vespa/searchlib/features/CMakeLists.txt @@ -16,7 +16,6 @@ vespa_add_library(searchlib_features OBJECT distance_calculator_bundle.cpp distancefeature.cpp distancetopathfeature.cpp - documenttestutils.cpp dotproductfeature.cpp element_completeness_feature.cpp element_similarity_feature.cpp diff --git a/searchlib/src/vespa/searchlib/features/documenttestutils.cpp b/searchlib/src/vespa/searchlib/features/documenttestutils.cpp deleted file mode 100644 index 5962cb32573..00000000000 --- a/searchlib/src/vespa/searchlib/features/documenttestutils.cpp +++ /dev/null @@ -1,166 +0,0 @@ -// Copyright Vespa.ai. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. - - - -#include "utils.h" -#include <vespa/searchlib/fef/itablemanager.h> -#include <vespa/searchlib/fef/properties.h> -#include <vespa/searchlib/fef/itermdata.h> -#include <vespa/vespalib/util/stringfmt.h> -#include <vespa/vespalib/stllike/asciistream.h> - -#include <cmath> -#include <ostream> - -#include <vespa/vespalib/util/issue.h> -using vespalib::Issue; - -#include <vespa/log/log.h> -LOG_SETUP(".features.utils"); -using namespace search::fef; - -namespace search::features::util { - -feature_t -lookupConnectedness(const search::fef::IQueryEnvironment& env, uint32_t termId, feature_t fallback) -{ - if (termId == 0) { - return fallback; // no previous term - } - - const ITermData * data = env.getTerm(termId); - const ITermData * prev = env.getTerm(termId - 1); - if (data == nullptr || prev == nullptr) { - return fallback; // default value - } - return lookupConnectedness(env, data->getUniqueId(), prev->getUniqueId(), fallback); -} - -feature_t -lookupConnectedness(const search::fef::IQueryEnvironment& env, - uint32_t currUniqueId, uint32_t prevUniqueId, feature_t fallback) -{ - // Connectedness of 0.5 between term with unique id 2 and term with unique id 1 is represented as: - // [vespa.term.2.connexity: "1", vespa.term.2.connexity: "0.5"] - vespalib::asciistream os; - os << "vespa.term." << currUniqueId << ".connexity"; - Property p = env.getProperties().lookup(os.str()); - if (p.size() == 2) { - // we have a defined connectedness with the previous term - if (strToNum<uint32_t>(p.getAt(0)) == prevUniqueId) { - return strToNum<feature_t>(p.getAt(1)); - } - } - return fallback; -} - -feature_t -lookupSignificance(const search::fef::IQueryEnvironment& env, const ITermData& term, feature_t fallback) -{ - // Significance of 0.5 for term with unique id 1 is represented as: - // [vespa.term.1.significance: "0.5"] - vespalib::asciistream os; - os << "vespa.term." << term.getUniqueId() << ".significance"; - Property p = env.getProperties().lookup(os.str()); - if (p.found()) { - return strToNum<feature_t>(p.get()); - } - return fallback; -} - -feature_t -lookupSignificance(const search::fef::IQueryEnvironment& env, uint32_t termId, feature_t fallback) -{ - const ITermData* term = env.getTerm(termId); - if (term == nullptr) { - return fallback; - } - return lookupSignificance(env, *term, fallback); -} - -double -getRobertsonSparckJonesWeight(double docCount, double docsInCorpus) -{ - return std::log((docsInCorpus - docCount + 0.5)/(docCount + 0.5)); -} - -static const double N = 1000000.0; - -feature_t -getSignificance(double docFreq) -{ - if (docFreq < (1.0/N)) { - docFreq = 1.0/N; - } - if (docFreq > 1.0) { - docFreq = 1.0; - } - double d = std::log(docFreq)/std::log(1.0/N); - return 0.5 + 0.5 * d; -#if 0 - double n = docFreq * N; - n = (n == 0) ? 1 : (n > N ? N : n); - double a = getRobertsonSparckJonesWeight(1, N + 1); - double b = getRobertsonSparckJonesWeight(N + 1, N + 1); - double w = getRobertsonSparckJonesWeight(n, N + 1); - return ((w - b)/(a - b)); -#endif -} - -feature_t -getSignificance(const search::fef::ITermData& termData) -{ - using FRA = search::fef::ITermFieldRangeAdapter; - double df = 0; - for (FRA iter(termData); iter.valid(); iter.next()) { - df = std::max(df, iter.get().getDocFreq()); - } - - feature_t signif = getSignificance(df); - LOG(debug, "getSignificance %e %f [ %e %f ] = %e", df, df, df * N, df * N, signif); - return signif; -} - -const search::fef::Table * -lookupTable(const search::fef::IIndexEnvironment & env, const vespalib::string & featureName, - const vespalib::string & table, const vespalib::string & fieldName, const vespalib::string & fallback) -{ - vespalib::string tn1 = env.getProperties().lookup(featureName, table).get(fallback); - vespalib::string tn2 = env.getProperties().lookup(featureName, table, fieldName).get(tn1); - const search::fef::Table * retval = env.getTableManager().getTable(tn2); - if (retval == nullptr) { - LOG(warning, "Could not find the %s '%s' to be used for field '%s' in feature '%s'", - table.c_str(), tn2.c_str(), fieldName.c_str(), featureName.c_str()); - } - return retval; -} - -const search::fef::ITermData * -getTermByLabel(const search::fef::IQueryEnvironment &env, const vespalib::string &label) -{ - // Labeling the query item with unique id '5' with the label 'foo' - // is represented as: [vespa.label.foo.id: "5"] - vespalib::asciistream os; - os << "vespa.label." << label << ".id"; - Property p = env.getProperties().lookup(os.str()); - if (!p.found()) { - return 0; - } - uint32_t uid = strToNum<uint32_t>(p.get()); - if (uid == 0) { - Issue::report("Query label '%s' was attached to invalid unique id: '%s'", - label.c_str(), p.get().c_str()); - return 0; - } - for (uint32_t i(0), m(env.getNumTerms()); i < m; ++i) { - const ITermData *term = env.getTerm(i); - if (term->getUniqueId() == uid) { - return term; - } - } - Issue::report("Query label '%s' was attached to non-existing unique id: '%s'", - label.c_str(), p.get().c_str()); - return 0; -} - -} diff --git a/searchlib/src/vespa/searchlib/features/utils.cpp b/searchlib/src/vespa/searchlib/features/utils.cpp index 92758c58262..0c3bbcb0ffa 100644 --- a/searchlib/src/vespa/searchlib/features/utils.cpp +++ b/searchlib/src/vespa/searchlib/features/utils.cpp @@ -1,7 +1,20 @@ // Copyright Vespa.ai. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. #include "utils.hpp" +#include <vespa/searchlib/fef/itablemanager.h> +#include <vespa/searchlib/fef/properties.h> +#include <vespa/searchlib/fef/itermdata.h> +#include <vespa/vespalib/util/stringfmt.h> +#include <vespa/vespalib/util/issue.h> #include <charconv> +#include <cmath> +#include <ostream> + +#include <vespa/log/log.h> +LOG_SETUP(".features.utils"); + +using vespalib::Issue; +using namespace search::fef; namespace search::features::util { @@ -40,4 +53,146 @@ template <> int16_t strToNum<int16_t>(vespalib::stringref str) { return strToIn template <> int32_t strToNum<int32_t>(vespalib::stringref str) { return strToInt<int32_t>(str); } template <> int64_t strToNum<int64_t>(vespalib::stringref str) { return strToInt<int64_t>(str); } +feature_t +lookupConnectedness(const search::fef::IQueryEnvironment& env, uint32_t termId, feature_t fallback) +{ + if (termId == 0) { + return fallback; // no previous term + } + + const ITermData * data = env.getTerm(termId); + const ITermData * prev = env.getTerm(termId - 1); + if (data == nullptr || prev == nullptr) { + return fallback; // default value + } + return lookupConnectedness(env, data->getUniqueId(), prev->getUniqueId(), fallback); +} + +feature_t +lookupConnectedness(const search::fef::IQueryEnvironment& env, + uint32_t currUniqueId, uint32_t prevUniqueId, feature_t fallback) +{ + // Connectedness of 0.5 between term with unique id 2 and term with unique id 1 is represented as: + // [vespa.term.2.connexity: "1", vespa.term.2.connexity: "0.5"] + vespalib::asciistream os; + os << "vespa.term." << currUniqueId << ".connexity"; + Property p = env.getProperties().lookup(os.str()); + if (p.size() == 2) { + // we have a defined connectedness with the previous term + if (strToNum<uint32_t>(p.getAt(0)) == prevUniqueId) { + return strToNum<feature_t>(p.getAt(1)); + } + } + return fallback; +} + +feature_t +lookupSignificance(const search::fef::IQueryEnvironment& env, const ITermData& term, feature_t fallback) +{ + // Significance of 0.5 for term with unique id 1 is represented as: + // [vespa.term.1.significance: "0.5"] + vespalib::asciistream os; + os << "vespa.term." << term.getUniqueId() << ".significance"; + Property p = env.getProperties().lookup(os.str()); + if (p.found()) { + return strToNum<feature_t>(p.get()); + } + return fallback; +} + +feature_t +lookupSignificance(const search::fef::IQueryEnvironment& env, uint32_t termId, feature_t fallback) +{ + const ITermData* term = env.getTerm(termId); + if (term == nullptr) { + return fallback; + } + return lookupSignificance(env, *term, fallback); +} + +double +getRobertsonSparckJonesWeight(double docCount, double docsInCorpus) +{ + return std::log((docsInCorpus - docCount + 0.5)/(docCount + 0.5)); +} + +static const double N = 1000000.0; + +feature_t +getSignificance(double docFreq) +{ + if (docFreq < (1.0/N)) { + docFreq = 1.0/N; + } + if (docFreq > 1.0) { + docFreq = 1.0; + } + double d = std::log(docFreq)/std::log(1.0/N); + return 0.5 + 0.5 * d; +#if 0 + double n = docFreq * N; + n = (n == 0) ? 1 : (n > N ? N : n); + double a = getRobertsonSparckJonesWeight(1, N + 1); + double b = getRobertsonSparckJonesWeight(N + 1, N + 1); + double w = getRobertsonSparckJonesWeight(n, N + 1); + return ((w - b)/(a - b)); +#endif +} + +feature_t +getSignificance(const search::fef::ITermData& termData) +{ + using FRA = search::fef::ITermFieldRangeAdapter; + double df = 0; + for (FRA iter(termData); iter.valid(); iter.next()) { + df = std::max(df, iter.get().getDocFreq()); + } + + feature_t signif = getSignificance(df); + LOG(debug, "getSignificance %e %f [ %e %f ] = %e", df, df, df * N, df * N, signif); + return signif; +} + +const search::fef::Table * +lookupTable(const search::fef::IIndexEnvironment & env, const vespalib::string & featureName, + const vespalib::string & table, const vespalib::string & fieldName, const vespalib::string & fallback) +{ + vespalib::string tn1 = env.getProperties().lookup(featureName, table).get(fallback); + vespalib::string tn2 = env.getProperties().lookup(featureName, table, fieldName).get(tn1); + const search::fef::Table * retval = env.getTableManager().getTable(tn2); + if (retval == nullptr) { + LOG(warning, "Could not find the %s '%s' to be used for field '%s' in feature '%s'", + table.c_str(), tn2.c_str(), fieldName.c_str(), featureName.c_str()); + } + return retval; +} + +const search::fef::ITermData * +getTermByLabel(const search::fef::IQueryEnvironment &env, const vespalib::string &label) +{ + // Labeling the query item with unique id '5' with the label 'foo' + // is represented as: [vespa.label.foo.id: "5"] + vespalib::asciistream os; + os << "vespa.label." << label << ".id"; + Property p = env.getProperties().lookup(os.str()); + if (!p.found()) { + return 0; + } + uint32_t uid = strToNum<uint32_t>(p.get()); + if (uid == 0) { + Issue::report("Query label '%s' was attached to invalid unique id: '%s'", + label.c_str(), p.get().c_str()); + return 0; + } + for (uint32_t i(0), m(env.getNumTerms()); i < m; ++i) { + const ITermData *term = env.getTerm(i); + if (term->getUniqueId() == uid) { + return term; + } + } + Issue::report("Query label '%s' was attached to non-existing unique id: '%s'", + label.c_str(), p.get().c_str()); + return 0; +} + } diff --git a/searchlib/src/vespa/searchlib/features/utils.h b/searchlib/src/vespa/searchlib/features/utils.h index b7b84013630..e90592fbe5d 100644 --- a/searchlib/src/vespa/searchlib/features/utils.h +++ b/searchlib/src/vespa/searchlib/features/utils.h @@ -71,22 +71,6 @@ inline feature_t getAsFeature<vespalib::stringref>(vespalib::stringref value) { return vespalib::hash2d(value); } - -/** - * This method inputs a value to cap to the range [capFloor, capCeil] and then normalize this - * value to the unit range [0, 1]. - * - * @param val The value to unit normalize. - * @param capFloor The minimum value of the cap range. - * @param capCeil The maximum value of the cap range. - * @return The unit normalized value. - */ -template <typename T> -T unitNormalize(const T &val, const T &capFloor, const T &capCeil) -{ - return (std::max(capFloor, std::min(capCeil, val)) - capFloor) / (capCeil - capFloor); -} - /** * Returns the normalized strength with which the given term is connected to the previous term in the query. * Uses the property map of the query environment to lookup this data. diff --git a/searchlib/src/vespa/searchlib/query/tree/queryreplicator.h b/searchlib/src/vespa/searchlib/query/tree/queryreplicator.h index 116e677d439..3da68db4c34 100644 --- a/searchlib/src/vespa/searchlib/query/tree/queryreplicator.h +++ b/searchlib/src/vespa/searchlib/query/tree/queryreplicator.h @@ -8,6 +8,7 @@ #include "queryvisitor.h" #include "string_term_vector.h" #include "termnodes.h" +#include <cassert> namespace search::query { @@ -29,8 +30,8 @@ public: private: void visitNodes(const std::vector<Node *> &nodes) { - for (size_t i = 0; i < nodes.size(); ++i) { - nodes[i]->accept(*this); + for (auto node : nodes) { + node->accept(*this); } } diff --git a/storage/src/tests/bucketdb/bucketmanagertest.cpp b/storage/src/tests/bucketdb/bucketmanagertest.cpp index 6aff5c52598..f41dae89eec 100644 --- a/storage/src/tests/bucketdb/bucketmanagertest.cpp +++ b/storage/src/tests/bucketdb/bucketmanagertest.cpp @@ -24,6 +24,7 @@ #include <vespa/vdslib/state/random.h> #include <vespa/vespalib/gtest/gtest.h> #include <vespa/vespalib/stllike/asciistream.h> +#include <vespa/vespalib/testkit/test_path.h> #include <vespa/config-stor-distribution.h> #include <future> @@ -130,7 +131,7 @@ void BucketManagerTest::setupTestEnvironment() { _config = StorageConfigSet::make_storage_node_config(); auto repo = std::make_shared<const DocumentTypeRepo>( - *ConfigGetter<DocumenttypesConfig>::getConfig("config-doctypes", FileSpec("../config-doctypes.cfg"))); + *ConfigGetter<DocumenttypesConfig>::getConfig("config-doctypes", FileSpec(TEST_PATH("../config-doctypes.cfg")))); _top = std::make_unique<DummyStorageLink>(); _node = std::make_unique<TestServiceLayerApp>(NodeIndex(0), _config->config_uri()); _node->setTypeRepo(repo); diff --git a/storage/src/tests/storageserver/documentapiconvertertest.cpp b/storage/src/tests/storageserver/documentapiconvertertest.cpp index 1eb6bf5dd9a..365b9efeff0 100644 --- a/storage/src/tests/storageserver/documentapiconvertertest.cpp +++ b/storage/src/tests/storageserver/documentapiconvertertest.cpp @@ -19,6 +19,7 @@ #include <vespa/storageapi/message/removelocation.h> #include <vespa/storageapi/message/stat.h> #include <vespa/vespalib/gtest/gtest.h> +#include <vespa/vespalib/testkit/test_path.h> #include <vespa/documentapi/messagebus/messages/testandsetcondition.h> using document::Bucket; @@ -71,7 +72,7 @@ struct DocumentApiConverterTest : Test { DocumentApiConverterTest() : _bucketResolver(std::make_shared<MockBucketResolver>()), - _repo(std::make_shared<DocumentTypeRepo>(readDocumenttypesConfig("../config-doctypes.cfg"))), + _repo(std::make_shared<DocumentTypeRepo>(readDocumenttypesConfig(TEST_PATH("../config-doctypes.cfg")))), _html_type(*_repo->getDocumentType("text/html")) { } diff --git a/storage/src/tests/storageserver/rpc/cluster_controller_rpc_api_service_test.cpp b/storage/src/tests/storageserver/rpc/cluster_controller_rpc_api_service_test.cpp index c3641b9bc56..e59f6d22080 100644 --- a/storage/src/tests/storageserver/rpc/cluster_controller_rpc_api_service_test.cpp +++ b/storage/src/tests/storageserver/rpc/cluster_controller_rpc_api_service_test.cpp @@ -121,7 +121,7 @@ struct SetStateFixture : FixtureBase { } static lib::ClusterStateBundle dummy_baseline_bundle_with_deferred_activation(bool deferred) { - return lib::ClusterStateBundle(lib::ClusterState("version:123 distributor:3 storage:3"), {}, deferred); + return {lib::ClusterState("version:123 distributor:3 storage:3"), {}, deferred}; } }; @@ -166,6 +166,16 @@ TEST_F(ClusterControllerApiRpcServiceTest, set_distribution_states_rpc_with_feed f.assert_request_received_and_propagated(bundle); } +TEST_F(ClusterControllerApiRpcServiceTest, can_receive_cluster_state_bundle_with_embedded_distribution_config) { + auto distr_cfg = lib::DistributionConfigBundle::of(lib::Distribution::getDefaultDistributionConfig(3, 14)); + SetStateFixture f; + lib::ClusterStateBundle bundle( + std::make_shared<const lib::ClusterState>("version:123 distributor:3 storage:3"), + {}, std::nullopt, std::move(distr_cfg), false); + + f.assert_request_received_and_propagated(bundle); +} + TEST_F(ClusterControllerApiRpcServiceTest, compressed_bundle_is_transparently_uncompressed) { SetStateFixture f; auto state_str = make_compressable_state_string(); diff --git a/storage/src/vespa/storage/storageserver/rpc/cluster_state_bundle_codec.h b/storage/src/vespa/storage/storageserver/rpc/cluster_state_bundle_codec.h index 81bd0897dbb..53b613ce2ab 100644 --- a/storage/src/vespa/storage/storageserver/rpc/cluster_state_bundle_codec.h +++ b/storage/src/vespa/storage/storageserver/rpc/cluster_state_bundle_codec.h @@ -21,8 +21,8 @@ class ClusterStateBundleCodec { public: virtual ~ClusterStateBundleCodec() = default; - virtual EncodedClusterStateBundle encode(const lib::ClusterStateBundle&) const = 0; - virtual std::shared_ptr<const lib::ClusterStateBundle> decode(const EncodedClusterStateBundle&) const = 0; + [[nodiscard]] virtual EncodedClusterStateBundle encode(const lib::ClusterStateBundle&) const = 0; + [[nodiscard]] virtual std::shared_ptr<const lib::ClusterStateBundle> decode(const EncodedClusterStateBundle&) const = 0; }; } diff --git a/storage/src/vespa/storage/storageserver/rpc/slime_cluster_state_bundle_codec.cpp b/storage/src/vespa/storage/storageserver/rpc/slime_cluster_state_bundle_codec.cpp index 38d3f929549..f143066ac34 100644 --- a/storage/src/vespa/storage/storageserver/rpc/slime_cluster_state_bundle_codec.cpp +++ b/storage/src/vespa/storage/storageserver/rpc/slime_cluster_state_bundle_codec.cpp @@ -1,9 +1,15 @@ // Copyright Vespa.ai. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. #include "slime_cluster_state_bundle_codec.h" +#include <vespa/config-stor-distribution.h> +#include <vespa/config/common/misc.h> +#include <vespa/config/configgen/configpayload.h> +#include <vespa/config/print/configdatabuffer.h> #include <vespa/document/bucket/fixed_bucket_spaces.h> -#include <vespa/vdslib/state/clusterstate.h> #include <vespa/vdslib/state/cluster_state_bundle.h> +#include <vespa/vdslib/state/clusterstate.h> +#include <vespa/vespalib/data/slime/array_traverser.h> +#include <vespa/vespalib/data/slime/object_traverser.h> #include <vespa/vespalib/data/slime/slime.h> #include <vespa/vespalib/stllike/asciistream.h> #include <vespa/vespalib/util/size_literals.h> @@ -17,6 +23,7 @@ using vespalib::compression::CompressionConfig; using vespalib::compression::decompress; using vespalib::compression::compress; using vespalib::Memory; +using DistributionConfigBuilder = storage::lib::Distribution::DistributionConfigBuilder; using namespace vespalib::slime; namespace storage::rpc { @@ -48,21 +55,94 @@ vespalib::string serialize_state(const lib::ClusterState& state) { return as.str(); } -const Memory StatesField("states"); const Memory BaselineField("baseline"); -const Memory SpacesField("spaces"); -const Memory DeferredActivationField("deferred-activation"); -const Memory FeedBlockField("feed-block"); const Memory BlockFeedInClusterField("block-feed-in-cluster"); +const Memory DeferredActivationField("deferred-activation"); const Memory DescriptionField("description"); +const Memory DistributionConfigField("distribution-config"); +const Memory FeedBlockField("feed-block"); +const Memory SpacesField("spaces"); +const Memory StatesField("states"); + +// Important: these conversion routines are NOT complete and NOT general! They are only to be used +// by code transitively used by unit tests that expect a particular type subset and "shape" of config. + +void convert_struct(const Inspector& in, Cursor& out); + +struct ConfigArrayConverter : ArrayTraverser { + Cursor& _out; + explicit ConfigArrayConverter(Cursor& out) noexcept: _out(out) {} + + void entry([[maybe_unused]] size_t idx, const Inspector& in) override { + assert(in.type().getId() == OBJECT::ID); + auto type = in["type"].asString(); + auto& value = in["value"]; + assert(value.valid()); + if (type == "int") { + _out.addLong(value.asLong()); + } else if (type == "bool") { + _out.addBool(value.asBool()); + } else if (type == "string") { + _out.addString(value.asString()); + } else if (type == "double") { + _out.addDouble(value.asDouble()); + } else if (type == "array") { + assert(value.type().getId() == ARRAY::ID); + ConfigArrayConverter arr_conv(_out.addArray()); + value.traverse(arr_conv); + } else if (type == "struct") { + convert_struct(value, _out.addObject()); + } else { + fprintf(stderr, "unknown array entry type '%s'\n", type.make_string().c_str()); + abort(); + } + } +}; + +struct ConfigObjectConverter : ObjectTraverser { + Cursor& _out; + explicit ConfigObjectConverter(Cursor& out) noexcept: _out(out) {} + + void field(const Memory& name, const Inspector& in) override { + assert(in.type().getId() == OBJECT::ID); + auto type = in["type"].asString(); + auto& value = in["value"]; + assert(value.valid()); + if (type == "int") { + _out.setLong(name, value.asLong()); + } else if (type == "bool") { + _out.setBool(name, value.asBool()); + } else if (type == "string") { + _out.setString(name, value.asString()); + } else if (type == "double") { + _out.setDouble(name, value.asDouble()); + } else if (type == "array") { + assert(value.type().getId() == ARRAY::ID); + ConfigArrayConverter arr_conv(_out.setArray(name)); + value.traverse(arr_conv); + } else if (type == "struct") { + convert_struct(value, _out.setObject(name)); + } else { + fprintf(stderr, "unknown struct entry type '%s'\n", type.make_string().c_str()); + abort(); + } + } +}; + +void convert_struct(const Inspector& in, Cursor& out) { + ConfigObjectConverter conv(out); + in.traverse(conv); +} +void convert_to_config_payload(const Inspector& in, Cursor& out) { + convert_struct(in["configPayload"], out); } +} // anon ns + // Only used from unit tests; the cluster controller encodes all bundles // we decode in practice. -EncodedClusterStateBundle SlimeClusterStateBundleCodec::encode( - const lib::ClusterStateBundle& bundle) const -{ +EncodedClusterStateBundle SlimeClusterStateBundleCodec::encode(const lib::ClusterStateBundle& bundle) const { vespalib::Slime slime; Cursor& root = slime.setObject(); if (bundle.deferredActivation()) { @@ -81,6 +161,15 @@ EncodedClusterStateBundle SlimeClusterStateBundleCodec::encode( feed_block.setString(DescriptionField, bundle.feed_block()->description()); } + if (bundle.has_distribution_config()) { + Cursor& distr_root = root.setObject(DistributionConfigField); + ::config::ConfigDataBuffer buf; + bundle.distribution_config_bundle()->config().serialize(buf); + // There is no way in C++ to directly serialize to the actual payload format we expect to + // deserialize, so we have to manually convert the type-annotated config snapshot :I + convert_to_config_payload(buf.slimeObject().get(), distr_root); + } + OutputBuf out_buf(4_Ki); BinaryFormat::encode(slime, out_buf); ConstBufferRef to_compress(out_buf.getBuf().getData(), out_buf.getBuf().getDataLen()); @@ -130,7 +219,7 @@ std::shared_ptr<const lib::ClusterStateBundle> SlimeClusterStateBundleCodec::dec BinaryFormat::decode(Memory(uncompressed.getData(), uncompressed.getDataLen()), slime); Inspector& root = slime.get(); Inspector& states = root[StatesField]; - lib::ClusterState baseline(states[BaselineField].asString().make_string()); + auto baseline = std::make_shared<lib::ClusterState>(states[BaselineField].asString().make_string()); Inspector& spaces = states[SpacesField]; lib::ClusterStateBundle::BucketSpaceStateMapping space_states; @@ -138,16 +227,21 @@ std::shared_ptr<const lib::ClusterStateBundle> SlimeClusterStateBundleCodec::dec spaces.traverse(inserter); const bool deferred_activation = root[DeferredActivationField].asBool(); // Defaults to false if not set. + std::shared_ptr<const lib::DistributionConfigBundle> distribution_config; + std::optional<lib::ClusterStateBundle::FeedBlock> feed_block; Inspector& fb = root[FeedBlockField]; if (fb.valid()) { - lib::ClusterStateBundle::FeedBlock feed_block(fb[BlockFeedInClusterField].asBool(), - fb[DescriptionField].asString().make_string()); - return std::make_shared<lib::ClusterStateBundle>(baseline, std::move(space_states), feed_block, deferred_activation); + feed_block = lib::ClusterStateBundle::FeedBlock(fb[BlockFeedInClusterField].asBool(), + fb[DescriptionField].asString().make_string()); } - - // TODO add shared_ptr constructor for baseline? - return std::make_shared<lib::ClusterStateBundle>(baseline, std::move(space_states), deferred_activation); + Inspector& dc = root[DistributionConfigField]; + if (dc.valid()) { + auto raw_cfg = std::make_unique<DistributionConfigBuilder>(::config::ConfigPayload(dc)); + distribution_config = lib::DistributionConfigBundle::of(std::move(raw_cfg)); + } + return std::make_shared<lib::ClusterStateBundle>(std::move(baseline), std::move(space_states), std::move(feed_block), + std::move(distribution_config), deferred_activation); } } diff --git a/vdslib/src/tests/distribution/distributiontest.cpp b/vdslib/src/tests/distribution/distributiontest.cpp index 2038c5706f5..9d590c72559 100644 --- a/vdslib/src/tests/distribution/distributiontest.cpp +++ b/vdslib/src/tests/distribution/distributiontest.cpp @@ -11,11 +11,14 @@ #include <vespa/vespalib/gtest/gtest.h> #include <vespa/vespalib/io/fileutil.h> #include <vespa/vespalib/stllike/lexical_cast.h> +#include <vespa/vespalib/test/test_data.h> +#include <vespa/vespalib/testkit/test_path.h> #include <vespa/vespalib/text/stringtokenizer.h> #include <vespa/vespalib/util/benchmark_timer.h> #include <vespa/vespalib/util/size_literals.h> #include <gmock/gmock.h> #include <chrono> +#include <filesystem> #include <fstream> #include <iterator> #include <thread> @@ -30,7 +33,35 @@ T readConfig(const config::ConfigUri & uri) return *config::ConfigGetter<T>::getConfig(uri.getConfigId(), uri.getContext()); } -TEST(DistributionTest, test_verify_java_distributions) +class DistributionTest : public ::testing::Test, public vespalib::test::TestData<DistributionTest> { +protected: + DistributionTest(); + ~DistributionTest() override; + static void SetUpTestSuite(); + static void TearDownTestSuite(); +}; + +DistributionTest::DistributionTest() + : ::testing::Test(), + vespalib::test::TestData<DistributionTest>() +{ +} + +DistributionTest::~DistributionTest() = default; + +void +DistributionTest::SetUpTestSuite() +{ + setup_test_data(TEST_PATH("distribution/testdata"), "distribution-testdata"); +} + +void +DistributionTest::TearDownTestSuite() +{ + tear_down_test_data(); +} + +TEST_F(DistributionTest, test_verify_java_distributions) { std::vector<std::string> tests; tests.push_back("capacity"); @@ -39,16 +70,19 @@ TEST(DistributionTest, test_verify_java_distributions) tests.push_back("retired"); for (uint32_t i=0; i<tests.size(); ++i) { std::string test = tests[i]; + std::string java_cfg(source_testdata() + "/java_" + test + ".cfg"); + std::string java_testfile(source_testdata() + "/java_" + test + ".distribution"); + std::string cpp_testfile(build_testdata() + "/cpp_" + test + ".distribution"); std::string mystate; { - std::ifstream in("distribution/testdata/java_" + test + ".state"); + std::ifstream in(source_testdata() + "/java_" + test + ".state"); in >> mystate; in.close(); } ClusterState state(mystate); Distribution distr(readConfig<vespa::config::content::StorDistributionConfig>( - config::ConfigUri("file:distribution/testdata/java_" + test + ".cfg"))); - std::ofstream of("distribution/testdata/cpp_" + test + ".distribution"); + config::ConfigUri("file:" + java_cfg))); + std::ofstream of(cpp_testfile); long maxBucket = 1; long mask = 0; @@ -74,10 +108,13 @@ TEST(DistributionTest, test_verify_java_distributions) } of.close(); std::ostringstream cmd; - cmd << "diff -u " << "distribution/testdata/cpp_" << test << ".distribution " - << "distribution/testdata/java_" << test << ".distribution"; + + cmd << "diff -u " << cpp_testfile << " " << java_testfile; int result = system(cmd.str().c_str()); - EXPECT_EQ(0, result) << "Failed distribution sync test: " + test; + EXPECT_EQ(0, result) << "Failed distribution sync test: " << test; + if (result == 0) { + std::filesystem::remove(cpp_testfile); + } } } @@ -186,9 +223,9 @@ auto readFile(const std::string & filename) { return buf; } -TEST(DistributionTest, test_verify_java_distributions_2) +TEST_F(DistributionTest, test_verify_java_distributions_2) { - vespalib::DirectoryList files(vespalib::listDirectory("distribution/testdata")); + vespalib::DirectoryList files(vespalib::listDirectory(source_testdata())); for (uint32_t i=0, n=files.size(); i<n; ++i) { size_t pos = files[i].find(".java.results"); if (pos == vespalib::string::npos || pos + 13 != files[i].size()) { @@ -200,14 +237,14 @@ TEST(DistributionTest, test_verify_java_distributions_2) using namespace vespalib::slime; vespalib::Slime slime; - auto buf = readFile("distribution/testdata/" + files[i]); + auto buf = readFile(source_testdata() + "/" + files[i]); auto size = JsonFormat::decode({&buf[0], buf.size()}, slime); if (size == 0) { std::cerr << "\n\nSize of " << files[i] << " is 0. Maybe is not generated yet? Taking a 5 second nap!"; std::this_thread::sleep_for(std::chrono::seconds(5)); - buf = readFile("distribution/testdata/" + files[i]); + buf = readFile(source_testdata() + "/" + files[i]); size = JsonFormat::decode({&buf[0], buf.size()}, slime); if (size == 0) { @@ -245,12 +282,12 @@ TEST(DistributionTest, test_verify_java_distributions_2) } } -TEST(DistributionTest, test_unchanged_distribution) +TEST_F(DistributionTest, test_unchanged_distribution) { ClusterState state("distributor:10 storage:10"); Distribution distr(Distribution::getDefaultDistributionConfig(3, 10)); - std::ifstream in("distribution/testdata/41-distributordistribution"); + std::ifstream in(source_testdata() + "/41-distributordistribution"); for (unsigned i = 0; i < 64_Ki; i++) { uint16_t node = distr.getIdealDistributorNode(state, document::BucketId(16, i), "u"); @@ -353,7 +390,7 @@ std::vector<uint16_t> createNodeCountList(const std::string& source, std::vector EXPECT_EQ(exp123, cnt123); \ } -TEST(DistributionTest, test_down) +TEST_F(DistributionTest, test_down) { ASSERT_BUCKET_NODE_COUNTS( MyTest().state("storage:10 .4.s:m .5.s:m .6.s:d .7.s:d .9.s:r") @@ -366,7 +403,7 @@ TEST(DistributionTest, test_down) "0:+ 1:+ 2:+ 3:+ 8:+ 9:+"); } -TEST(DistributionTest, test_serialize_deserialize) +TEST_F(DistributionTest, test_serialize_deserialize) { MyTest t1; MyTest t2; @@ -374,7 +411,7 @@ TEST(DistributionTest, test_serialize_deserialize) EXPECT_EQ(t1.getNodeCounts(), t2.getNodeCounts()); } -TEST(DistributionTest, test_initializing) +TEST_F(DistributionTest, test_initializing) { ASSERT_BUCKET_NODE_COUNTS( MyTest().state("distributor:3 .0.s:i .1.s:i .2.s:i") @@ -383,7 +420,7 @@ TEST(DistributionTest, test_initializing) "0:+ 1:+ 2:+"); } -TEST(DistributionTest, testHighSplitBit) +TEST_F(DistributionTest, testHighSplitBit) { // Only 3 nodes of 10 are up => all copies should end on the 3 nodes and // none on the down nodes @@ -422,7 +459,7 @@ TEST(DistributionTest, testHighSplitBit) EXPECT_EQ(ost1.str(), ost2.str()); } -TEST(DistributionTest, test_distribution) +TEST_F(DistributionTest, test_distribution) { const int min_buckets = 64_Ki; const int max_buckets = 64_Ki; @@ -478,7 +515,7 @@ TEST(DistributionTest, test_distribution) } } -TEST(DistributionTest, test_move) +TEST_F(DistributionTest, test_move) { // This test is quite fragile, it will break if the ideal state algorithm is // changed in such a way that Bucket 0x8b4f67ae remains on node 0 and 1 if @@ -511,7 +548,7 @@ TEST(DistributionTest, test_move) EXPECT_EQ(1, int(it-diff.begin())); } -TEST(DistributionTest, test_move_constraints) +TEST_F(DistributionTest, test_move_constraints) { ClusterState clusterState("storage:10"); @@ -598,7 +635,7 @@ TEST(DistributionTest, test_move_constraints) } } -TEST(DistributionTest, test_distribution_bits) +TEST_F(DistributionTest, test_distribution_bits) { ClusterState state1("bits:16 distributor:10"); ClusterState state2("bits:19 distributor:10"); @@ -619,7 +656,7 @@ TEST(DistributionTest, test_distribution_bits) EXPECT_NE(ost1.str(), ost2.str()); } -TEST(DistributionTest, test_redundancy_hierarchical_distribution) +TEST_F(DistributionTest, test_redundancy_hierarchical_distribution) { ClusterState state("storage:10 distributor:10"); @@ -633,7 +670,7 @@ TEST(DistributionTest, test_redundancy_hierarchical_distribution) } } -TEST(DistributionTest, test_hierarchical_distribution) +TEST_F(DistributionTest, test_hierarchical_distribution) { std::string distConfig( "redundancy 4\n" @@ -683,7 +720,7 @@ TEST(DistributionTest, test_hierarchical_distribution) EXPECT_EQ(expectedMains, mainNode); } -TEST(DistributionTest, test_group_capacity) +TEST_F(DistributionTest, test_group_capacity) { std::string distConfig( "redundancy 1\n" @@ -727,7 +764,7 @@ TEST(DistributionTest, test_group_capacity) EXPECT_EQ(1000 - group0count, group1count); } -TEST(DistributionTest, test_hierarchical_no_redistribution) +TEST_F(DistributionTest, test_hierarchical_no_redistribution) { std::string distConfig( "redundancy 2\n" @@ -871,7 +908,7 @@ std::string groupConfig("group[3]\n" "group[2].nodes[2].index 5\n"); } -TEST(DistributionTest, test_active_per_group) +TEST_F(DistributionTest, test_active_per_group) { using IndexList = Distribution::IndexList; // Disabled feature @@ -946,7 +983,7 @@ TEST(DistributionTest, test_active_per_group) } } -TEST(DistributionTest, test_hierarchical_distribute_less_than_redundancy) +TEST_F(DistributionTest, test_hierarchical_distribute_less_than_redundancy) { Distribution distr("redundancy 4\nactive_per_leaf_group true\n" + groupConfig); ClusterState state("storage:6"); @@ -974,7 +1011,7 @@ TEST(DistributionTest, test_hierarchical_distribute_less_than_redundancy) } } -TEST(DistributionTest, wildcard_top_level_distribution_gives_expected_node_results) { +TEST_F(DistributionTest, wildcard_top_level_distribution_gives_expected_node_results) { std::string raw_config = R"(redundancy 2 initial_redundancy 2 ensure_primary_persisted true @@ -1062,7 +1099,7 @@ std::string generate_state_with_n_nodes_up(int n_nodes) { } -TEST(DistributionTest, DISABLED_benchmark_ideal_state_for_many_groups) { +TEST_F(DistributionTest, DISABLED_benchmark_ideal_state_for_many_groups) { const int n_groups = 150; Distribution distr(generate_config_with_n_1node_groups(n_groups)); ClusterState state(generate_state_with_n_nodes_up(n_groups)); @@ -1075,7 +1112,7 @@ TEST(DistributionTest, DISABLED_benchmark_ideal_state_for_many_groups) { fprintf(stderr, "%.10f seconds\n", min_time); } -TEST(DistributionTest, control_size_of_IndexList) { +TEST_F(DistributionTest, control_size_of_IndexList) { EXPECT_EQ(24u, sizeof(Distribution::IndexList)); } diff --git a/vespa-athenz/src/main/java/com/yahoo/vespa/athenz/identityprovider/api/DefaultSignedIdentityDocument.java b/vespa-athenz/src/main/java/com/yahoo/vespa/athenz/identityprovider/api/DefaultSignedIdentityDocument.java deleted file mode 100644 index 9f37e3f4613..00000000000 --- a/vespa-athenz/src/main/java/com/yahoo/vespa/athenz/identityprovider/api/DefaultSignedIdentityDocument.java +++ /dev/null @@ -1,14 +0,0 @@ -// Copyright Vespa.ai. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -package com.yahoo.vespa.athenz.identityprovider.api; - -public record DefaultSignedIdentityDocument(String signature, int signingKeyVersion, int documentVersion, - String data, IdentityDocument identityDocument) implements SignedIdentityDocument { - - public DefaultSignedIdentityDocument { - identityDocument = EntityBindingsMapper.fromIdentityDocumentData(data); - } - - public DefaultSignedIdentityDocument(String signature, int signingKeyVersion, int documentVersion, String data) { - this(signature,signingKeyVersion,documentVersion, data, null); - } -} diff --git a/vespa-athenz/src/main/java/com/yahoo/vespa/athenz/identityprovider/api/EntityBindingsMapper.java b/vespa-athenz/src/main/java/com/yahoo/vespa/athenz/identityprovider/api/EntityBindingsMapper.java index ac620d2f6d4..123995721e9 100644 --- a/vespa-athenz/src/main/java/com/yahoo/vespa/athenz/identityprovider/api/EntityBindingsMapper.java +++ b/vespa-athenz/src/main/java/com/yahoo/vespa/athenz/identityprovider/api/EntityBindingsMapper.java @@ -5,9 +5,10 @@ import com.fasterxml.jackson.core.JsonProcessingException; import com.fasterxml.jackson.databind.ObjectMapper; import com.fasterxml.jackson.datatype.jsr310.JavaTimeModule; import com.yahoo.vespa.athenz.api.AthenzService; -import com.yahoo.vespa.athenz.identityprovider.api.bindings.DefaultSignedIdentityDocumentEntity; +import com.yahoo.vespa.athenz.identityprovider.api.bindings.V4SignedIdentityDocumentEntity; import com.yahoo.vespa.athenz.identityprovider.api.bindings.IdentityDocumentEntity; import com.yahoo.vespa.athenz.identityprovider.api.bindings.SignedIdentityDocumentEntity; +import com.yahoo.vespa.athenz.identityprovider.api.bindings.V5SignedIdentityDocumentEntity; import com.yahoo.vespa.athenz.utils.AthenzIdentities; import com.yahoo.yolean.Exceptions; @@ -52,22 +53,32 @@ public class EntityBindingsMapper { } public static SignedIdentityDocument toSignedIdentityDocument(SignedIdentityDocumentEntity entity) { - if (entity instanceof DefaultSignedIdentityDocumentEntity docEntity) { - return new DefaultSignedIdentityDocument(docEntity.signature(), - docEntity.signingKeyVersion(), - docEntity.documentVersion(), - docEntity.data()); + if (entity instanceof V4SignedIdentityDocumentEntity docEntity) { + return new V4SignedIdentityDocument(docEntity.signature(), + docEntity.signingKeyVersion(), + docEntity.documentVersion(), + docEntity.data()); + } else if (entity instanceof V5SignedIdentityDocumentEntity docEntity) { + return new V5SignedIdentityDocument(docEntity.signature(), + docEntity.signingKeyVersion(), + docEntity.documentVersion(), + docEntity.data()); } else { throw new IllegalArgumentException("Unknown signed identity document type: " + entity.getClass().getName()); } } public static SignedIdentityDocumentEntity toSignedIdentityDocumentEntity(SignedIdentityDocument model) { - if (model instanceof DefaultSignedIdentityDocument defaultModel){ - return new DefaultSignedIdentityDocumentEntity(defaultModel.signature(), - defaultModel.signingKeyVersion(), - defaultModel.documentVersion(), - defaultModel.data()); + if (model instanceof V4SignedIdentityDocument defaultModel) { + return new V4SignedIdentityDocumentEntity(defaultModel.signature(), + defaultModel.v4SigningKeyVersion(), + defaultModel.documentVersion(), + defaultModel.data()); + } else if (model instanceof V5SignedIdentityDocument defaultModel){ + return new V5SignedIdentityDocumentEntity(defaultModel.signature(), + defaultModel.signingKeyVersion(), + defaultModel.documentVersion(), + defaultModel.data()); } else { throw new IllegalArgumentException("Unsupported model type: " + model.getClass().getName()); } diff --git a/vespa-athenz/src/main/java/com/yahoo/vespa/athenz/identityprovider/api/SignedIdentityDocument.java b/vespa-athenz/src/main/java/com/yahoo/vespa/athenz/identityprovider/api/SignedIdentityDocument.java index 39629d878db..56b67694af7 100644 --- a/vespa-athenz/src/main/java/com/yahoo/vespa/athenz/identityprovider/api/SignedIdentityDocument.java +++ b/vespa-athenz/src/main/java/com/yahoo/vespa/athenz/identityprovider/api/SignedIdentityDocument.java @@ -8,12 +8,14 @@ package com.yahoo.vespa.athenz.identityprovider.api; */ public interface SignedIdentityDocument { - int DEFAULT_DOCUMENT_VERSION = 4; + int LEGACY_DOCUMENT_VERSION = 4; + int DEFAULT_DOCUMENT_VERSION = 5; default boolean outdated() { return documentVersion() < DEFAULT_DOCUMENT_VERSION; } IdentityDocument identityDocument(); String signature(); - int signingKeyVersion(); + String signingKeyVersion(); int documentVersion(); + String data(); } diff --git a/vespa-athenz/src/main/java/com/yahoo/vespa/athenz/identityprovider/api/V4SignedIdentityDocument.java b/vespa-athenz/src/main/java/com/yahoo/vespa/athenz/identityprovider/api/V4SignedIdentityDocument.java new file mode 100644 index 00000000000..36836786da3 --- /dev/null +++ b/vespa-athenz/src/main/java/com/yahoo/vespa/athenz/identityprovider/api/V4SignedIdentityDocument.java @@ -0,0 +1,19 @@ +// Copyright Vespa.ai. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.vespa.athenz.identityprovider.api; + +public record V4SignedIdentityDocument(String signature, int v4SigningKeyVersion, int documentVersion, + String data, IdentityDocument identityDocument) implements SignedIdentityDocument { + + public V4SignedIdentityDocument { + identityDocument = EntityBindingsMapper.fromIdentityDocumentData(data); + } + + public V4SignedIdentityDocument(String signature, int v4SigningKeyVersion, int documentVersion, String data) { + this(signature, v4SigningKeyVersion, documentVersion, data, null); + } + + @Override + public String signingKeyVersion() { + return Integer.toString(v4SigningKeyVersion); + } +} diff --git a/vespa-athenz/src/main/java/com/yahoo/vespa/athenz/identityprovider/api/V5SignedIdentityDocument.java b/vespa-athenz/src/main/java/com/yahoo/vespa/athenz/identityprovider/api/V5SignedIdentityDocument.java new file mode 100644 index 00000000000..644ca2eafb4 --- /dev/null +++ b/vespa-athenz/src/main/java/com/yahoo/vespa/athenz/identityprovider/api/V5SignedIdentityDocument.java @@ -0,0 +1,16 @@ +// Copyright Vespa.ai. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. + +package com.yahoo.vespa.athenz.identityprovider.api; + +public record V5SignedIdentityDocument(String signature, String signingKeyVersion, int documentVersion, + String data, IdentityDocument identityDocument) implements SignedIdentityDocument { + + + public V5SignedIdentityDocument { + identityDocument = EntityBindingsMapper.fromIdentityDocumentData(data); + } + + public V5SignedIdentityDocument(String signature, String signingKeyVersion, int documentVersion, String data) { + this(signature,signingKeyVersion,documentVersion, data, null); + } +} diff --git a/vespa-athenz/src/main/java/com/yahoo/vespa/athenz/identityprovider/api/bindings/SignedIdentityDocumentEntity.java b/vespa-athenz/src/main/java/com/yahoo/vespa/athenz/identityprovider/api/bindings/SignedIdentityDocumentEntity.java index d909849e9ce..dc2daa530e8 100644 --- a/vespa-athenz/src/main/java/com/yahoo/vespa/athenz/identityprovider/api/bindings/SignedIdentityDocumentEntity.java +++ b/vespa-athenz/src/main/java/com/yahoo/vespa/athenz/identityprovider/api/bindings/SignedIdentityDocumentEntity.java @@ -9,6 +9,7 @@ import com.fasterxml.jackson.databind.JavaType; import com.fasterxml.jackson.databind.annotation.JsonTypeIdResolver; import com.fasterxml.jackson.databind.jsontype.TypeIdResolver; import com.fasterxml.jackson.databind.type.TypeFactory; +import com.yahoo.vespa.athenz.identityprovider.api.SignedIdentityDocument; import java.io.IOException; import java.util.Objects; @@ -54,7 +55,12 @@ class SignedIdentityDocumentEntityTypeResolver implements TypeIdResolver { @Override public JavaType typeFromId(DatabindContext databindContext, String s) throws IOException { try { - Class<? extends SignedIdentityDocumentEntity> cls = DefaultSignedIdentityDocumentEntity.class; + int version = Integer.parseInt(s); + Class<? extends SignedIdentityDocumentEntity> cls = switch (version) { + case SignedIdentityDocument.LEGACY_DOCUMENT_VERSION -> V4SignedIdentityDocumentEntity.class; + case SignedIdentityDocument.DEFAULT_DOCUMENT_VERSION -> V5SignedIdentityDocumentEntity.class; + default -> throw new IllegalArgumentException("Unknown document version: " + version); + }; return TypeFactory.defaultInstance().constructSpecializedType(javaType,cls); } catch (NumberFormatException e) { throw new IllegalArgumentException("Unable to deserialize document with version: \"%s\"".formatted(s)); diff --git a/vespa-athenz/src/main/java/com/yahoo/vespa/athenz/identityprovider/api/bindings/DefaultSignedIdentityDocumentEntity.java b/vespa-athenz/src/main/java/com/yahoo/vespa/athenz/identityprovider/api/bindings/V4SignedIdentityDocumentEntity.java index 74fd43feb35..9c6af38377a 100644 --- a/vespa-athenz/src/main/java/com/yahoo/vespa/athenz/identityprovider/api/bindings/DefaultSignedIdentityDocumentEntity.java +++ b/vespa-athenz/src/main/java/com/yahoo/vespa/athenz/identityprovider/api/bindings/V4SignedIdentityDocumentEntity.java @@ -3,7 +3,7 @@ package com.yahoo.vespa.athenz.identityprovider.api.bindings; import com.fasterxml.jackson.annotation.JsonProperty; -public record DefaultSignedIdentityDocumentEntity( +public record V4SignedIdentityDocumentEntity( @JsonProperty("signature") String signature, @JsonProperty("signing-key-version") int signingKeyVersion, @JsonProperty("document-version") int documentVersion, diff --git a/vespa-athenz/src/main/java/com/yahoo/vespa/athenz/identityprovider/api/bindings/V5SignedIdentityDocumentEntity.java b/vespa-athenz/src/main/java/com/yahoo/vespa/athenz/identityprovider/api/bindings/V5SignedIdentityDocumentEntity.java new file mode 100644 index 00000000000..eece4b5f066 --- /dev/null +++ b/vespa-athenz/src/main/java/com/yahoo/vespa/athenz/identityprovider/api/bindings/V5SignedIdentityDocumentEntity.java @@ -0,0 +1,12 @@ +// Copyright Vespa.ai. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.vespa.athenz.identityprovider.api.bindings; + +import com.fasterxml.jackson.annotation.JsonProperty; + +public record V5SignedIdentityDocumentEntity( + @JsonProperty("signature") String signature, + @JsonProperty("signing-key-version") String signingKeyVersion, + @JsonProperty("document-version") int documentVersion, + @JsonProperty("data") String data) + implements SignedIdentityDocumentEntity { +} diff --git a/vespa-athenz/src/main/java/com/yahoo/vespa/athenz/identityprovider/client/IdentityDocumentSigner.java b/vespa-athenz/src/main/java/com/yahoo/vespa/athenz/identityprovider/client/IdentityDocumentSigner.java index 43f32a3bae7..392faaaa339 100644 --- a/vespa-athenz/src/main/java/com/yahoo/vespa/athenz/identityprovider/client/IdentityDocumentSigner.java +++ b/vespa-athenz/src/main/java/com/yahoo/vespa/athenz/identityprovider/client/IdentityDocumentSigner.java @@ -2,23 +2,14 @@ package com.yahoo.vespa.athenz.identityprovider.client; import com.yahoo.security.SignatureUtils; -import com.yahoo.vespa.athenz.api.AthenzIdentity; -import com.yahoo.vespa.athenz.identityprovider.api.DefaultSignedIdentityDocument; -import com.yahoo.vespa.athenz.identityprovider.api.IdentityDocument; -import com.yahoo.vespa.athenz.identityprovider.api.IdentityType; +import com.yahoo.vespa.athenz.identityprovider.api.V4SignedIdentityDocument; import com.yahoo.vespa.athenz.identityprovider.api.SignedIdentityDocument; -import com.yahoo.vespa.athenz.identityprovider.api.VespaUniqueInstanceId; -import java.nio.ByteBuffer; import java.security.GeneralSecurityException; import java.security.PrivateKey; import java.security.PublicKey; import java.security.Signature; -import java.security.SignatureException; -import java.time.Instant; import java.util.Base64; -import java.util.Set; -import java.util.TreeSet; import static java.nio.charset.StandardCharsets.UTF_8; @@ -42,17 +33,13 @@ public class IdentityDocumentSigner { } public boolean hasValidSignature(SignedIdentityDocument doc, PublicKey publicKey) { - if (doc instanceof DefaultSignedIdentityDocument signedDoc) { - try { - Signature signer = SignatureUtils.createVerifier(publicKey); - signer.initVerify(publicKey); - signer.update(signedDoc.data().getBytes(UTF_8)); - return signer.verify(Base64.getDecoder().decode(doc.signature())); - } catch (GeneralSecurityException e) { - throw new RuntimeException(e); - } - } else { - throw new IllegalArgumentException("Unknown identity document type: " + doc.getClass().getName()); + try { + Signature signer = SignatureUtils.createVerifier(publicKey); + signer.initVerify(publicKey); + signer.update(doc.data().getBytes(UTF_8)); + return signer.verify(Base64.getDecoder().decode(doc.signature())); + } catch (GeneralSecurityException e) { + throw new RuntimeException(e); } } } diff --git a/vespa-athenz/src/test/java/com/yahoo/vespa/athenz/identityprovider/client/IdentityDocumentSignerTest.java b/vespa-athenz/src/test/java/com/yahoo/vespa/athenz/identityprovider/client/IdentityDocumentSignerTest.java index 3845d9db5b2..3479350cf26 100644 --- a/vespa-athenz/src/test/java/com/yahoo/vespa/athenz/identityprovider/client/IdentityDocumentSignerTest.java +++ b/vespa-athenz/src/test/java/com/yahoo/vespa/athenz/identityprovider/client/IdentityDocumentSignerTest.java @@ -6,7 +6,7 @@ import com.yahoo.security.KeyUtils; import com.yahoo.vespa.athenz.api.AthenzIdentity; import com.yahoo.vespa.athenz.api.AthenzService; import com.yahoo.vespa.athenz.identityprovider.api.ClusterType; -import com.yahoo.vespa.athenz.identityprovider.api.DefaultSignedIdentityDocument; +import com.yahoo.vespa.athenz.identityprovider.api.V4SignedIdentityDocument; import com.yahoo.vespa.athenz.identityprovider.api.EntityBindingsMapper; import com.yahoo.vespa.athenz.identityprovider.api.IdentityDocument; import com.yahoo.vespa.athenz.identityprovider.api.IdentityType; @@ -53,7 +53,7 @@ public class IdentityDocumentSignerTest { String signature = signer.generateSignature(data, keyPair.getPrivate()); - SignedIdentityDocument signedIdentityDocument = new DefaultSignedIdentityDocument( + SignedIdentityDocument signedIdentityDocument = new V4SignedIdentityDocument( signature, KEY_VERSION, DEFAULT_DOCUMENT_VERSION, data); assertTrue(signer.hasValidSignature(signedIdentityDocument, keyPair.getPublic())); diff --git a/vespalib/src/tests/barrier/barrier_test.cpp b/vespalib/src/tests/barrier/barrier_test.cpp index eba57f5381f..18032d92ce0 100644 --- a/vespalib/src/tests/barrier/barrier_test.cpp +++ b/vespalib/src/tests/barrier/barrier_test.cpp @@ -1,6 +1,7 @@ // Copyright Vespa.ai. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. #include <vespa/vespalib/testkit/test_kit.h> #include <vespa/vespalib/util/barrier.h> +#include <vespa/vespalib/util/count_down_latch.h> using namespace vespalib; diff --git a/vespalib/src/tests/compression/compression_test.cpp b/vespalib/src/tests/compression/compression_test.cpp index 264f21aeefe..2257b57dc7e 100644 --- a/vespalib/src/tests/compression/compression_test.cpp +++ b/vespalib/src/tests/compression/compression_test.cpp @@ -4,6 +4,7 @@ #include <vespa/vespalib/stllike/string.h> #include <vespa/vespalib/util/compressor.h> #include <vespa/vespalib/data/databuffer.h> +#include <atomic> #include <vespa/log/log.h> LOG_SETUP("compression_test"); diff --git a/vespalib/src/tests/datastore/buffer_type/buffer_type_test.cpp b/vespalib/src/tests/datastore/buffer_type/buffer_type_test.cpp index f721eb6d95b..d2dcf99081b 100644 --- a/vespalib/src/tests/datastore/buffer_type/buffer_type_test.cpp +++ b/vespalib/src/tests/datastore/buffer_type/buffer_type_test.cpp @@ -2,6 +2,7 @@ #include <vespa/vespalib/datastore/buffer_type.h> #include <vespa/vespalib/testkit/test_kit.h> +#include <cassert> using namespace vespalib::datastore; diff --git a/vespalib/src/tests/net/send_fd/CMakeLists.txt b/vespalib/src/tests/net/send_fd/CMakeLists.txt index 4c46a773f5c..b4baaeb5855 100644 --- a/vespalib/src/tests/net/send_fd/CMakeLists.txt +++ b/vespalib/src/tests/net/send_fd/CMakeLists.txt @@ -4,5 +4,6 @@ vespa_add_executable(vespalib_send_fd_test_app TEST send_fd_test.cpp DEPENDS vespalib + GTest::gtest ) vespa_add_test(NAME vespalib_send_fd_test_app COMMAND vespalib_send_fd_test_app) diff --git a/vespalib/src/tests/net/send_fd/send_fd_test.cpp b/vespalib/src/tests/net/send_fd/send_fd_test.cpp index 59b9aacea07..8dc1235ff76 100644 --- a/vespalib/src/tests/net/send_fd/send_fd_test.cpp +++ b/vespalib/src/tests/net/send_fd/send_fd_test.cpp @@ -1,5 +1,5 @@ // Copyright Vespa.ai. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -#include <vespa/vespalib/testkit/test_kit.h> +#include <vespa/vespalib/gtest/gtest.h> #include <vespa/vespalib/testkit/time_bomb.h> #include <vespa/vespalib/net/selector.h> @@ -7,15 +7,19 @@ #include <vespa/vespalib/net/server_socket.h> #include <vespa/vespalib/net/socket_options.h> #include <vespa/vespalib/net/socket.h> +#include <vespa/vespalib/test/nexus.h> #include <vespa/vespalib/util/stringfmt.h> #include <vespa/vespalib/test/socket_options_verifier.h> -#include <thread> -#include <functional> #include <chrono> +#include <functional> +#include <latch> +#include <optional> +#include <thread> #include <unistd.h> #include <sys/stat.h> using namespace vespalib; +using vespalib::test::Nexus; vespalib::string read_bytes(SocketHandle &socket, size_t wanted_bytes) { char tmp[64]; @@ -23,7 +27,9 @@ vespalib::string read_bytes(SocketHandle &socket, size_t wanted_bytes) { while (result.size() < wanted_bytes) { size_t read_size = std::min(sizeof(tmp), wanted_bytes - result.size()); ssize_t read_result = socket.read(tmp, read_size); - ASSERT_GREATER(read_result, 0); + if (read_result <= 0) { + return result; + } result.append(tmp, read_result); } return result; @@ -34,14 +40,14 @@ void verify_socket_io(bool is_server, SocketHandle &socket) { vespalib::string client_message = "please pick up, I need to talk to you"; if(is_server) { ssize_t written = socket.write(server_message.data(), server_message.size()); - EXPECT_EQUAL(written, ssize_t(server_message.size())); + EXPECT_EQ(written, ssize_t(server_message.size())); vespalib::string read = read_bytes(socket, client_message.size()); - EXPECT_EQUAL(client_message, read); + EXPECT_EQ(client_message, read); } else { ssize_t written = socket.write(client_message.data(), client_message.size()); - EXPECT_EQUAL(written, ssize_t(client_message.size())); + EXPECT_EQ(written, ssize_t(client_message.size())); vespalib::string read = read_bytes(socket, server_message.size()); - EXPECT_EQUAL(server_message, read); + EXPECT_EQ(server_message, read); } } @@ -79,10 +85,10 @@ void send_fd(SocketHandle &socket, SocketHandle fd) { int *fd_dst = (int *) (void *) CMSG_DATA(hdr); fd_dst[0] = fd.get(); ssize_t res = sendmsg(socket.get(), &msg, 0); - ASSERT_EQUAL(res, 1); + ASSERT_EQ(res, 1); } -SocketHandle recv_fd(SocketHandle &socket) { +void recv_fd(SocketHandle &socket, std::optional<SocketHandle>& result) { struct msghdr msg = {}; char tag = '*'; struct iovec data; @@ -94,36 +100,62 @@ SocketHandle recv_fd(SocketHandle &socket) { msg.msg_control = buf; msg.msg_controllen = sizeof(buf); ssize_t res = recvmsg(socket.get(), &msg, 0); - ASSERT_EQUAL(res, 1); + ASSERT_EQ(res, 1); struct cmsghdr *hdr = CMSG_FIRSTHDR(&msg); bool type_ok = ((hdr->cmsg_level == SOL_SOCKET) && (hdr->cmsg_type == SCM_RIGHTS)); ASSERT_TRUE(type_ok); int *fd_src = (int *) (void *) CMSG_DATA(hdr); fprintf(stderr, "got fd: %d\n", fd_src[0]); - return SocketHandle(fd_src[0]); + result = SocketHandle(fd_src[0]); } //----------------------------------------------------------------------------- -TEST_MT_FFF("require that an open socket (handle) can be passed over a unix domain socket", 3, - ServerSocket("tcp/0"), ServerSocket("ipc/file:my_socket"), TimeBomb(60)) -{ - if (thread_id == 0) { // server - SocketHandle socket = accept(f1); - TEST_DO(verify_socket_io(true, socket)); // server side - TEST_BARRIER(); - } else if (thread_id == 1) { // proxy - SocketHandle server_socket = connect(f1); - SocketHandle client_socket = accept(f2); - send_fd(client_socket, std::move(server_socket)); - TEST_BARRIER(); - } else { // client - SocketHandle proxy_socket = connect(f2); - SocketHandle socket = recv_fd(proxy_socket); - TEST_DO(verify_socket_io(false, socket)); // client side - TEST_BARRIER(); +namespace { + +class WaitLatch { + std::latch& _latch; +public: + explicit WaitLatch(std::latch& latch) noexcept + : _latch(latch) + { } + ~WaitLatch() { _latch.arrive_and_wait(); } +}; + +} + +TEST(SendFdTest, require_that_an_open_socket_handle_can_be_passed_over_a_unix_domain_socket) +{ + constexpr size_t num_threads = 3; + ServerSocket f1("tcp/0"); + ServerSocket f2("ipc/file:my_socket"); + std::latch latch(num_threads); + TimeBomb f3(60); + auto task = [&f1,&f2,&latch](Nexus& ctx) { + auto thread_id = ctx.thread_id(); + if (thread_id == 0) { // server + SocketHandle socket = accept(f1); + WaitLatch wait(latch); + SCOPED_TRACE("verify socket io server side"); + verify_socket_io(true, socket); // server side + } else if (thread_id == 1) { // proxy + SocketHandle server_socket = connect(f1); + SocketHandle client_socket = accept(f2); + WaitLatch wait(latch); + ASSERT_NO_FATAL_FAILURE(send_fd(client_socket, std::move(server_socket))); + } else { // client + SocketHandle proxy_socket = connect(f2); + std::optional<SocketHandle> socket; + WaitLatch wait(latch); + ASSERT_NO_FATAL_FAILURE(recv_fd(proxy_socket, socket)); + ASSERT_TRUE(socket.has_value()); + SCOPED_TRACE("verify socket io client side"); + verify_socket_io(false, socket.value()); // client side + } + }; + Nexus::run(num_threads, task); } -TEST_MAIN() { TEST_RUN_ALL(); } +GTEST_MAIN_RUN_ALL_TESTS() diff --git a/vespalib/src/tests/net/socket/CMakeLists.txt b/vespalib/src/tests/net/socket/CMakeLists.txt index d20720971d2..49c4ca20ba3 100644 --- a/vespalib/src/tests/net/socket/CMakeLists.txt +++ b/vespalib/src/tests/net/socket/CMakeLists.txt @@ -4,6 +4,7 @@ vespa_add_executable(vespalib_socket_test_app TEST socket_test.cpp DEPENDS vespalib + GTest::gtest ) vespa_add_test(NAME vespalib_socket_test_app COMMAND vespalib_socket_test_app) vespa_add_executable(vespalib_socket_server_app diff --git a/vespalib/src/tests/net/socket/socket_test.cpp b/vespalib/src/tests/net/socket/socket_test.cpp index b0708388cd6..64f337c03f6 100644 --- a/vespalib/src/tests/net/socket/socket_test.cpp +++ b/vespalib/src/tests/net/socket/socket_test.cpp @@ -1,11 +1,12 @@ // Copyright Vespa.ai. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -#include <vespa/vespalib/testkit/test_kit.h> +#include <vespa/vespalib/gtest/gtest.h> #include <vespa/vespalib/testkit/time_bomb.h> #include <vespa/vespalib/net/selector.h> #include <vespa/vespalib/net/socket_spec.h> #include <vespa/vespalib/net/server_socket.h> #include <vespa/vespalib/net/socket_options.h> #include <vespa/vespalib/net/socket.h> +#include <vespa/vespalib/test/nexus.h> #include <vespa/vespalib/util/stringfmt.h> #include <vespa/vespalib/test/socket_options_verifier.h> #include <thread> @@ -14,19 +15,55 @@ #include <sys/stat.h> using namespace vespalib; +using vespalib::test::Nexus; -bool ipv4_enabled = false; -bool ipv6_enabled = false; +class SocketTest : public ::testing::Test { +protected: + static bool ipv4_enabled; + static bool ipv6_enabled; -int my_inet() { + SocketTest(); + ~SocketTest() override; + static void SetUpTestSuite(); + static void TearDownTestSuite(); + static int my_inet(); +}; + +bool SocketTest::ipv4_enabled = false; +bool SocketTest::ipv6_enabled = false; + +SocketTest::SocketTest() + : testing::Test() +{ +} + +SocketTest::~SocketTest() = default; + +void +SocketTest::SetUpTestSuite() +{ + auto list = SocketAddress::resolve(4080); + for (const auto &addr : list) { + (void) addr; + ipv4_enabled |= addr.is_ipv4(); + ipv6_enabled |= addr.is_ipv6(); + } + ASSERT_TRUE(ipv4_enabled || ipv6_enabled) << "tcp/ip support not detected"; +} + +void +SocketTest::TearDownTestSuite() +{ +} + +int +SocketTest::my_inet() +{ if (ipv6_enabled) { return AF_INET6; - } - if (ipv4_enabled) { + } else { return AF_INET; } - TEST_ERROR("tcp/ip support not detected"); - return AF_UNIX; } bool is_socket(const vespalib::string &path) { @@ -52,8 +89,8 @@ void remove_file(const vespalib::string &path) { void replace_file(const vespalib::string &path, const vespalib::string &data) { remove_file(path); int fd = creat(path.c_str(), 0600); - ASSERT_NOT_EQUAL(fd, -1); - ASSERT_EQUAL(write(fd, data.data(), data.size()), ssize_t(data.size())); + ASSERT_NE(fd, -1); + ASSERT_EQ(write(fd, data.data(), data.size()), ssize_t(data.size())); close(fd); } @@ -96,14 +133,14 @@ void verify_socket_io(bool is_server, SocketHandle &socket) { vespalib::string client_message = "please pick up, I need to talk to you"; if(is_server) { ssize_t written = socket.write(server_message.data(), server_message.size()); - EXPECT_EQUAL(written, ssize_t(server_message.size())); + EXPECT_EQ(written, ssize_t(server_message.size())); vespalib::string read = read_bytes(socket, client_message.size()); - EXPECT_EQUAL(client_message, read); + EXPECT_EQ(client_message, read); } else { ssize_t written = socket.write(client_message.data(), client_message.size()); - EXPECT_EQUAL(written, ssize_t(client_message.size())); + EXPECT_EQ(written, ssize_t(client_message.size())); vespalib::string read = read_bytes(socket, server_message.size()); - EXPECT_EQUAL(server_message, read); + EXPECT_EQ(server_message, read); } } @@ -122,22 +159,22 @@ SocketHandle connect_sockets(bool is_server, ServerSocket &server_socket) { //----------------------------------------------------------------------------- -TEST("my local address") { +TEST_F(SocketTest, my_local_address) +{ auto list = SocketAddress::resolve(4080); fprintf(stderr, "resolve(4080):\n"); for (const auto &addr: list) { EXPECT_TRUE(addr.is_wildcard()); EXPECT_TRUE(addr.is_ipv4() || addr.is_ipv6()); - ipv4_enabled |= addr.is_ipv4(); - ipv6_enabled |= addr.is_ipv6(); EXPECT_TRUE(!addr.is_ipc()); EXPECT_TRUE(!addr.is_abstract()); - EXPECT_EQUAL(addr.port(), 4080); + EXPECT_EQ(addr.port(), 4080); fprintf(stderr, " %s (%s)\n", addr.spec().c_str(), get_meta(addr).c_str()); } } -TEST("yahoo.com address") { +TEST_F(SocketTest, yahoo_com_address) +{ auto list = SocketAddress::resolve(80, "yahoo.com"); fprintf(stderr, "resolve(80, 'yahoo.com'):\n"); for (const auto &addr: list) { @@ -145,80 +182,102 @@ TEST("yahoo.com address") { EXPECT_TRUE(addr.is_ipv4() || addr.is_ipv6()); EXPECT_TRUE(!addr.is_ipc()); EXPECT_TRUE(!addr.is_abstract()); - EXPECT_EQUAL(addr.port(), 80); + EXPECT_EQ(addr.port(), 80); fprintf(stderr, " %s (%s)\n", addr.spec().c_str(), get_meta(addr).c_str()); } } -TEST("ipc address (path)") { +TEST_F(SocketTest, ipc_address_with_path) +{ auto addr = SocketAddress::from_path("my_socket"); EXPECT_TRUE(!addr.is_ipv4()); EXPECT_TRUE(!addr.is_ipv6()); EXPECT_TRUE(addr.is_ipc()); EXPECT_TRUE(!addr.is_abstract()); EXPECT_TRUE(!addr.is_wildcard()); - EXPECT_EQUAL(addr.port(), -1); - EXPECT_EQUAL(vespalib::string("my_socket"), addr.path()); + EXPECT_EQ(addr.port(), -1); + EXPECT_EQ(vespalib::string("my_socket"), addr.path()); EXPECT_TRUE(addr.name().empty()); fprintf(stderr, "from_path(my_socket)\n"); fprintf(stderr, " %s (%s)\n", addr.spec().c_str(), get_meta(addr).c_str()); } -TEST("ipc address (name)") { +TEST_F(SocketTest, ipc_address_with_name) +{ auto addr = SocketAddress::from_name("my_socket"); EXPECT_TRUE(!addr.is_ipv4()); EXPECT_TRUE(!addr.is_ipv6()); EXPECT_TRUE(addr.is_ipc()); EXPECT_TRUE(addr.is_abstract()); EXPECT_TRUE(!addr.is_wildcard()); - EXPECT_EQUAL(addr.port(), -1); + EXPECT_EQ(addr.port(), -1); EXPECT_TRUE(addr.path().empty()); - EXPECT_EQUAL(vespalib::string("my_socket"), addr.name()); + EXPECT_EQ(vespalib::string("my_socket"), addr.name()); fprintf(stderr, "from_path(my_socket)\n"); fprintf(stderr, " %s (%s)\n", addr.spec().c_str(), get_meta(addr).c_str()); } -TEST("local client/server addresses") { +TEST_F(SocketTest, local_client_and_server_addresses) { auto spec = SocketSpec("tcp/123"); auto client = spec.client_address(); auto server = spec.server_address(); EXPECT_TRUE(!client.is_wildcard()); - EXPECT_EQUAL(client.port(), 123); + EXPECT_EQ(client.port(), 123); EXPECT_TRUE(server.is_wildcard()); - EXPECT_EQUAL(server.port(), 123); + EXPECT_EQ(server.port(), 123); fprintf(stderr, "client(tcp/123): %s (%s)\n", client.spec().c_str(), get_meta(client).c_str()); fprintf(stderr, "server(tcp/123): %s (%s)\n", server.spec().c_str(), get_meta(server).c_str()); } -TEST_MT_FF("require that basic socket io works", 2, ServerSocket("tcp/0"), TimeBomb(60)) { - bool is_server = (thread_id == 0); - SocketHandle socket = connect_sockets(is_server, f1); - TEST_DO(verify_socket_io(is_server, socket)); -} - -TEST_MT_FF("require that basic unix domain socket io works (path)", 2, - ServerSocket("ipc/file:my_socket"), TimeBomb(60)) +TEST_F(SocketTest, require_that_basic_socket_io_works) +{ + constexpr size_t num_threads = 2; + ServerSocket f1("tcp/0"); + TimeBomb f2(60); + auto task = [&f1](Nexus& ctx) { + bool is_server = (ctx.thread_id() == 0); + SocketHandle socket = connect_sockets(is_server, f1); + verify_socket_io(is_server, socket); + }; + Nexus::run(num_threads, task); +} + +TEST_F(SocketTest, require_that_basic_unix_domain_socket_io_works_with_path) +{ + constexpr size_t num_threads = 2; + ServerSocket f1("ipc/file:my_socket"); + TimeBomb f2(60); + auto task = [&f1](Nexus& ctx) { + bool is_server = (ctx.thread_id() == 0); + SocketHandle socket = connect_sockets(is_server, f1); + verify_socket_io(is_server, socket); + }; + Nexus::run(num_threads, task); +} + +TEST_F(SocketTest, require_that_server_accept_can_be_interrupted) +{ + constexpr size_t num_threads = 2; + ServerSocket f1("tcp/0"); + TimeBomb f2(60); + auto task = [&f1](Nexus& ctx) { + bool is_server = (ctx.thread_id() == 0); + if (is_server) { + fprintf(stderr, "--> calling accept\n"); + SocketHandle socket = f1.accept(); + fprintf(stderr, "<-- accept returned\n"); + EXPECT_TRUE(!socket.valid()); + } else { + std::this_thread::sleep_for(std::chrono::milliseconds(20)); + fprintf(stderr, "--- closing server socket\n"); + f1.shutdown(); + } + }; + Nexus::run(num_threads, task); +} + +TEST_F(SocketTest, require_that_socket_file_is_removed_by_server_socket_when_destructed) { - bool is_server = (thread_id == 0); - SocketHandle socket = connect_sockets(is_server, f1); - TEST_DO(verify_socket_io(is_server, socket)); -} - -TEST_MT_FF("require that server accept can be interrupted", 2, ServerSocket("tcp/0"), TimeBomb(60)) { - bool is_server = (thread_id == 0); - if (is_server) { - fprintf(stderr, "--> calling accept\n"); - SocketHandle socket = f1.accept(); - fprintf(stderr, "<-- accept returned\n"); - EXPECT_TRUE(!socket.valid()); - } else { - std::this_thread::sleep_for(std::chrono::milliseconds(20)); - fprintf(stderr, "--- closing server socket\n"); - f1.shutdown(); - } -} - -TEST("require that socket file is removed by server socket when destructed") { remove_file("my_socket"); ServerSocket server("ipc/file:my_socket"); EXPECT_TRUE(server.valid()); @@ -227,7 +286,8 @@ TEST("require that socket file is removed by server socket when destructed") { EXPECT_TRUE(!is_socket("my_socket")); } -TEST("require that socket file is only removed on destruction if it is a socket") { +TEST_F(SocketTest, require_that_socket_file_is_only_removed_on_destruction_if_it_is_a_socket) +{ remove_file("my_socket"); ServerSocket server("ipc/file:my_socket"); EXPECT_TRUE(server.valid()); @@ -238,7 +298,8 @@ TEST("require that socket file is only removed on destruction if it is a socket" remove_file("my_socket"); } -TEST("require that a server socket will fail to listen to a path that is already a regular file") { +TEST_F(SocketTest, require_that_a_server_socket_will_fail_to_listen_to_a_path_that_is_already_a_regular_file) +{ replace_file("my_socket", "hello\n"); ServerSocket server("ipc/file:my_socket"); EXPECT_TRUE(!server.valid()); @@ -247,7 +308,8 @@ TEST("require that a server socket will fail to listen to a path that is already remove_file("my_socket"); } -TEST("require that a server socket will fail to listen to a path that is already taken by another server") { +TEST_F(SocketTest, require_that_a_server_socket_will_fail_to_listen_to_a_path_that_is_already_taken_by_another_server) +{ remove_file("my_socket"); ServerSocket server1("ipc/file:my_socket"); ServerSocket server2("ipc/file:my_socket"); @@ -258,7 +320,8 @@ TEST("require that a server socket will fail to listen to a path that is already EXPECT_TRUE(!is_socket("my_socket")); } -TEST("require that a server socket will remove an old socket file if it cannot be connected to") { +TEST_F(SocketTest, require_that_a_server_socket_will_remove_an_old_socket_file_if_it_cannot_be_connected_to) +{ remove_file("my_socket"); { SocketHandle server_handle = SocketAddress::from_path("my_socket").listen(); @@ -272,15 +335,21 @@ TEST("require that a server socket will remove an old socket file if it cannot b } #ifdef __linux__ -TEST_MT_FF("require that basic unix domain socket io works (name)", 2, - ServerSocket(make_string("ipc/name:my_socket-%d", int(getpid()))), TimeBomb(60)) +TEST_F(SocketTest, require_that_basic_unix_domain_socket_io_works_with_name) +{ + constexpr size_t num_threads = 2; + ServerSocket f1(make_string("ipc/name:my_socket-%d", int(getpid()))); + TimeBomb f2(60); + auto task = [&f1](Nexus& ctx) { + bool is_server = (ctx.thread_id() == 0); + SocketHandle socket = connect_sockets(is_server, f1); + verify_socket_io(is_server, socket); + }; + Nexus::run(num_threads, task); +} + +TEST_F(SocketTest, require_that_two_server_sockets_cannot_have_the_same_abstract_unix_domain_socket_name) { - bool is_server = (thread_id == 0); - SocketHandle socket = connect_sockets(is_server, f1); - TEST_DO(verify_socket_io(is_server, socket)); -} - -TEST("require that two server sockets cannot have the same abstract unix domain socket name") { vespalib::string spec = make_string("ipc/name:my_socket-%d", int(getpid())); ServerSocket server1(spec); ServerSocket server2(spec); @@ -288,7 +357,8 @@ TEST("require that two server sockets cannot have the same abstract unix domain EXPECT_TRUE(!server2.valid()); } -TEST("require that abstract socket names are freed when the server socket is destructed") { +TEST_F(SocketTest, require_that_abstract_socket_names_are_freed_when_the_server_socket_is_destructed) +{ vespalib::string spec = make_string("ipc/name:my_socket-%d", int(getpid())); ServerSocket server1(spec); EXPECT_TRUE(server1.valid()); @@ -297,100 +367,162 @@ TEST("require that abstract socket names are freed when the server socket is des EXPECT_TRUE(server2.valid()); } -TEST("require that abstract sockets do not have socket files") { +TEST_F(SocketTest, require_that_abstract_sockets_do_not_have_socket_files) +{ vespalib::string name = make_string("my_socket-%d", int(getpid())); ServerSocket server(SocketSpec::from_name(name)); EXPECT_TRUE(server.valid()); EXPECT_TRUE(!is_socket(name)); - EXPECT_TRUE(!is_file(name)); + EXPECT_TRUE(!is_file(name)); } -TEST_MT_FFF("require that abstract and file-based unix domain sockets are not in conflict", 4, - ServerSocket(make_string("ipc/file:my_socket-%d", int(getpid()))), - ServerSocket(make_string("ipc/name:my_socket-%d", int(getpid()))), TimeBomb(60)) +TEST_F(SocketTest, require_that_abstract_and_file_based_unix_domain_sockets_are_not_in_conflict) { - bool is_server = ((thread_id % 2) == 0); - ServerSocket &server_socket = ((thread_id / 2) == 0) ? f1 : f2; - SocketHandle socket = connect_sockets(is_server, server_socket); - TEST_DO(verify_socket_io(is_server, socket)); + constexpr size_t num_threads = 4; + ServerSocket f1(make_string("ipc/file:my_socket-%d", int(getpid()))); + ServerSocket f2(make_string("ipc/name:my_socket-%d", int(getpid()))); + TimeBomb f3(60); + auto task = [&f1,&f2](Nexus& ctx) { + auto thread_id = ctx.thread_id(); + bool is_server = ((thread_id % 2) == 0); + ServerSocket &server_socket = ((thread_id / 2) == 0) ? f1 : f2; + SocketHandle socket = connect_sockets(is_server, server_socket); + verify_socket_io(is_server, socket); + }; + Nexus::run(num_threads, task); } #endif -TEST("require that sockets can be set blocking and non-blocking") { +TEST_F(SocketTest, require_that_sockets_can_be_set_blocking_and_non_blocking) +{ SocketHandle handle(socket(my_inet(), SOCK_STREAM, 0)); test::SocketOptionsVerifier verifier(handle.get()); EXPECT_TRUE(!SocketOptions::set_blocking(-1, true)); EXPECT_TRUE(handle.set_blocking(true)); - TEST_DO(verifier.verify_blocking(true)); + { + SCOPED_TRACE("verify blocking true"); + verifier.verify_blocking(true); + } EXPECT_TRUE(handle.set_blocking(false)); - TEST_DO(verifier.verify_blocking(false)); + { + SCOPED_TRACE("verify blocking false"); + verifier.verify_blocking(false); + } } -TEST("require that server sockets use non-blocking underlying socket") { +TEST_F(SocketTest, require_that_server_sockets_use_non_blocking_underlying_socket) +{ ServerSocket tcp_server("tcp/0"); ServerSocket ipc_server("ipc/file:my_socket"); test::SocketOptionsVerifier tcp_verifier(tcp_server.get_fd()); test::SocketOptionsVerifier ipc_verifier(ipc_server.get_fd()); - TEST_DO(tcp_verifier.verify_blocking(false)); - TEST_DO(ipc_verifier.verify_blocking(false)); + { + SCOPED_TRACE("verify tcp nonblocking"); + tcp_verifier.verify_blocking(false); + } + { + SCOPED_TRACE("verify ipc nonblocking"); + ipc_verifier.verify_blocking(false); + } } -TEST("require that tcp nodelay can be enabled and disabled") { +TEST_F(SocketTest, require_that_tcp_nodelay_can_be_enabled_and_disabled) +{ SocketHandle handle(socket(my_inet(), SOCK_STREAM, 0)); test::SocketOptionsVerifier verifier(handle.get()); EXPECT_TRUE(!SocketOptions::set_nodelay(-1, true)); EXPECT_TRUE(handle.set_nodelay(true)); - TEST_DO(verifier.verify_nodelay(true)); + { + SCOPED_TRACE("verify nodelay true"); + verifier.verify_nodelay(true); + } EXPECT_TRUE(handle.set_nodelay(false)); - TEST_DO(verifier.verify_nodelay(false)); + { + SCOPED_TRACE("verify nodelay false"); + verifier.verify_nodelay(false); + } } -TEST("require that reuse addr can be set and cleared") { +TEST_F(SocketTest, require_that_reuse_addr_can_be_set_and_cleared) +{ SocketHandle handle(socket(my_inet(), SOCK_STREAM, 0)); test::SocketOptionsVerifier verifier(handle.get()); EXPECT_TRUE(!SocketOptions::set_reuse_addr(-1, true)); EXPECT_TRUE(handle.set_reuse_addr(true)); - TEST_DO(verifier.verify_reuse_addr(true)); + { + SCOPED_TRACE("verify reuse addr true"); + verifier.verify_reuse_addr(true); + } EXPECT_TRUE(handle.set_reuse_addr(false)); - TEST_DO(verifier.verify_reuse_addr(false)); + { + SCOPED_TRACE("verify reuse addr false"); + verifier.verify_reuse_addr(false); + } } -TEST("require that ipv6_only can be set and cleared") { +TEST_F(SocketTest, require_that_ipv6_only_can_be_set_and_cleared) +{ if (ipv6_enabled) { SocketHandle handle(socket(my_inet(), SOCK_STREAM, 0)); test::SocketOptionsVerifier verifier(handle.get()); EXPECT_TRUE(!SocketOptions::set_ipv6_only(-1, true)); EXPECT_TRUE(handle.set_ipv6_only(true)); - TEST_DO(verifier.verify_ipv6_only(true)); + { + SCOPED_TRACE("verify ipv6 only true"); + verifier.verify_ipv6_only(true); + } EXPECT_TRUE(handle.set_ipv6_only(false)); - TEST_DO(verifier.verify_ipv6_only(false)); + { + SCOPED_TRACE("verify ipv6 only false"); + verifier.verify_ipv6_only(false); + } } else { fprintf(stderr, "WARNING: skipping ipv6_only test since ipv6 is disabled"); } } -TEST("require that tcp keepalive can be set and cleared") { +TEST_F(SocketTest, require_that_tcp_keepalive_can_be_set_and_cleared) +{ SocketHandle handle(socket(my_inet(), SOCK_STREAM, 0)); test::SocketOptionsVerifier verifier(handle.get()); EXPECT_TRUE(!SocketOptions::set_keepalive(-1, true)); EXPECT_TRUE(handle.set_keepalive(true)); - TEST_DO(verifier.verify_keepalive(true)); + { + SCOPED_TRACE("verify keepalive true"); + verifier.verify_keepalive(true); + } EXPECT_TRUE(handle.set_keepalive(false)); - TEST_DO(verifier.verify_keepalive(false)); + { + SCOPED_TRACE("verify keepalive false"); + verifier.verify_keepalive(false); + } } -TEST("require that tcp lingering can be adjusted") { +TEST_F(SocketTest, require_that_tcp_lingering_can_be_adjusted) +{ SocketHandle handle(socket(my_inet(), SOCK_STREAM, 0)); test::SocketOptionsVerifier verifier(handle.get()); EXPECT_TRUE(!SocketOptions::set_linger(-1, true, 0)); EXPECT_TRUE(handle.set_linger(true, 0)); - TEST_DO(verifier.verify_linger(true, 0)); + { + SCOPED_TRACE("verify linger true 0"); + verifier.verify_linger(true, 0); + } EXPECT_TRUE(handle.set_linger(true, 10)); - TEST_DO(verifier.verify_linger(true, 10)); + { + SCOPED_TRACE("verify linger true 10"); + verifier.verify_linger(true, 10); + } EXPECT_TRUE(handle.set_linger(false, 0)); - TEST_DO(verifier.verify_linger(false, 0)); + { + SCOPED_TRACE("verify linger false 0"); + verifier.verify_linger(false, 0); + } EXPECT_TRUE(handle.set_linger(false, 10)); - TEST_DO(verifier.verify_linger(false, 0)); + { + SCOPED_TRACE("verify linger false 0 (overridden)"); + verifier.verify_linger(false, 0); + } } SocketHandle connect_async(const SocketAddress &addr) { @@ -411,7 +543,10 @@ SocketHandle connect_async(const SocketAddress &addr) { ctx.handle = addr.connect_async(); EXPECT_TRUE(ctx.handle.valid()); test::SocketOptionsVerifier verifier(ctx.handle.get()); - TEST_DO(verifier.verify_blocking(false)); + { + SCOPED_TRACE("verify blocking false"); + verifier.verify_blocking(false); + } if (ctx.handle.valid()) { selector.add(ctx.handle.get(), ctx, true, true); while (!ctx.connect_done) { @@ -420,23 +555,30 @@ SocketHandle connect_async(const SocketAddress &addr) { } selector.remove(ctx.handle.get()); } - EXPECT_EQUAL(ctx.error, 0); + EXPECT_EQ(ctx.error, 0); return std::move(ctx.handle); } -TEST_MT_FF("require that async connect pattern works", 2, ServerSocket("tcp/0"), TimeBomb(60)) { - if (thread_id == 0) { - SocketHandle socket = f1.accept(); - EXPECT_TRUE(socket.valid()); - TEST_DO(verify_socket_io(true, socket)); - } else { - SocketAddress addr = SocketSpec::from_port(f1.address().port()).client_address(); - SocketHandle socket = connect_async(addr); - socket.set_blocking(true); - TEST_DO(verify_socket_io(false, socket)); - // TEST_DO(connect_async(SocketAddress::select_remote(80, "www.yahoo.com"))); - // TEST_DO(connect_async(SocketAddress::select_remote(85, "myinternalhost"))); - } -} - -TEST_MAIN() { TEST_RUN_ALL(); } +TEST_F(SocketTest, require_that_async_connect_pattern_works) +{ + constexpr size_t num_threads = 2; + ServerSocket f1("tcp/0"); + TimeBomb f2(60); + auto task = [&f1](Nexus& ctx) { + if (ctx.thread_id() == 0) { + SocketHandle socket = f1.accept(); + EXPECT_TRUE(socket.valid()); + SCOPED_TRACE("verify socket io true"); + verify_socket_io(true, socket); + } else { + SocketAddress addr = SocketSpec::from_port(f1.address().port()).client_address(); + SocketHandle socket = connect_async(addr); + socket.set_blocking(true); + SCOPED_TRACE("verify socket io false"); + verify_socket_io(false, socket); + } + }; + Nexus::run(num_threads, task); +} + +GTEST_MAIN_RUN_ALL_TESTS() diff --git a/vespalib/src/tests/net/tls/auto_reloading_tls_crypto_engine/CMakeLists.txt b/vespalib/src/tests/net/tls/auto_reloading_tls_crypto_engine/CMakeLists.txt index 6bbf0189862..87e13f14875 100644 --- a/vespalib/src/tests/net/tls/auto_reloading_tls_crypto_engine/CMakeLists.txt +++ b/vespalib/src/tests/net/tls/auto_reloading_tls_crypto_engine/CMakeLists.txt @@ -4,6 +4,7 @@ vespa_add_executable(vespalib_net_tls_auto_reloading_tls_crypto_engine_test_app auto_reloading_tls_crypto_engine_test.cpp DEPENDS vespalib + GTest::gtest ) vespa_add_test(NAME vespalib_net_tls_auto_reloading_tls_crypto_engine_test_app COMMAND vespalib_net_tls_auto_reloading_tls_crypto_engine_test_app) diff --git a/vespalib/src/tests/net/tls/auto_reloading_tls_crypto_engine/auto_reloading_tls_crypto_engine_test.cpp b/vespalib/src/tests/net/tls/auto_reloading_tls_crypto_engine/auto_reloading_tls_crypto_engine_test.cpp index b6efb66c8bb..ed20dd6bcf4 100644 --- a/vespalib/src/tests/net/tls/auto_reloading_tls_crypto_engine/auto_reloading_tls_crypto_engine_test.cpp +++ b/vespalib/src/tests/net/tls/auto_reloading_tls_crypto_engine/auto_reloading_tls_crypto_engine_test.cpp @@ -1,15 +1,17 @@ // Copyright Vespa.ai. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +#include <vespa/vespalib/gtest/gtest.h> #include <vespa/vespalib/io/fileutil.h> #include <vespa/vespalib/net/tls/auto_reloading_tls_crypto_engine.h> #include <vespa/vespalib/net/tls/statistics.h> #include <vespa/vespalib/net/tls/transport_security_options.h> #include <vespa/vespalib/net/tls/transport_security_options_reading.h> #include <vespa/vespalib/net/tls/impl/openssl_tls_context_impl.h> -#include <vespa/vespalib/testkit/test_kit.h> +#include <vespa/vespalib/testkit/test_path.h> #include <vespa/vespalib/testkit/time_bomb.h> #include <openssl/ssl.h> #include <filesystem> +#include <fstream> using namespace vespalib; using namespace vespalib::net::tls; @@ -89,6 +91,42 @@ void write_file(vespalib::stringref path, vespalib::stringref data) { f.write(data.data(), data.size(), 0); } +class AutoReloadingTlsCryptoEngineTest : public ::testing::Test +{ +protected: + AutoReloadingTlsCryptoEngineTest(); + ~AutoReloadingTlsCryptoEngineTest() override; + static void SetUpTestSuite(); + static void TearDownTestSuite(); +}; + +AutoReloadingTlsCryptoEngineTest::AutoReloadingTlsCryptoEngineTest() + : testing::Test() +{ +} + +AutoReloadingTlsCryptoEngineTest::~AutoReloadingTlsCryptoEngineTest() = default; + +void +AutoReloadingTlsCryptoEngineTest::SetUpTestSuite() +{ + std::ofstream test_config("test_config.json"); + test_config << "{\n" << + " \"files\":{\n" << + " \"private-key\": \"" + TEST_PATH("test_key.pem") << "\",\n" << + " \"ca-certificates\": \"" + TEST_PATH("test_ca.pem") << "\",\n" << + " \"certificates\": \"test_cert.pem\"\n" << + " }\n" << + "}" << std::endl; + test_config.close(); +} + +void +AutoReloadingTlsCryptoEngineTest::TearDownTestSuite() +{ + std::filesystem::remove("test_config.json"); +} + struct Fixture { std::unique_ptr<AutoReloadingTlsCryptoEngine> engine; explicit Fixture(AutoReloadingTlsCryptoEngine::TimeInterval reload_interval, @@ -114,9 +152,13 @@ struct Fixture { } }; -TEST_FF("Config reloading transitively loads updated files", Fixture(50ms), TimeBomb(60)) { +TEST_F(AutoReloadingTlsCryptoEngineTest, config_reloading_transitively_loads_updated_files) +{ + Fixture f1(50ms); + TimeBomb f2(60); + auto current_certs = f1.current_cert_chain(); - ASSERT_EQUAL(cert1_pem, current_certs); + ASSERT_EQ(cert1_pem, current_certs); write_file("test_cert.pem.tmp", cert2_pem); std::filesystem::rename(std::filesystem::path("test_cert.pem.tmp"), std::filesystem::path("test_cert.pem")); // We expect this to be an atomic rename under the hood @@ -129,15 +171,25 @@ TEST_FF("Config reloading transitively loads updated files", Fixture(50ms), Time // If the config is never reloaded, test will go boom. } -TEST_FF("Shutting down auto-reloading engine immediately stops background thread", Fixture(600s), TimeBomb(60)) { +TEST_F(AutoReloadingTlsCryptoEngineTest, shutting_down_auto_reloading_engine_immediately_stops_background_thread) +{ + Fixture f1(600s); + TimeBomb f2(60); // This passes just from not having the TimeBomb blow up. } -TEST_FF("Authorization mode is propagated to engine", Fixture(50ms, AuthorizationMode::LogOnly), TimeBomb(60)) { - EXPECT_EQUAL(AuthorizationMode::LogOnly, f1.current_authorization_mode()); +TEST_F(AutoReloadingTlsCryptoEngineTest, authorization_mode_is_propagated_to_engine) +{ + Fixture f1(50ms, AuthorizationMode::LogOnly); + TimeBomb f2(60); + EXPECT_EQ(AuthorizationMode::LogOnly, f1.current_authorization_mode()); } -TEST_FF("Config reload failure increments failure statistic", Fixture(50ms), TimeBomb(60)) { +TEST_F(AutoReloadingTlsCryptoEngineTest, config_reload_failure_increments_failure_statistic) +{ + Fixture f1(50ms); + TimeBomb f2(60); + auto before = ConfigStatistics::get().snapshot(); write_file("test_cert.pem.tmp", "Broken file oh no :("); @@ -148,4 +200,4 @@ TEST_FF("Config reload failure increments failure statistic", Fixture(50ms), Tim } } -TEST_MAIN() { TEST_RUN_ALL(); } +GTEST_MAIN_RUN_ALL_TESTS() diff --git a/vespalib/src/tests/net/tls/auto_reloading_tls_crypto_engine/test_config.json b/vespalib/src/tests/net/tls/auto_reloading_tls_crypto_engine/test_config.json deleted file mode 100644 index 2b2322d928f..00000000000 --- a/vespalib/src/tests/net/tls/auto_reloading_tls_crypto_engine/test_config.json +++ /dev/null @@ -1,7 +0,0 @@ -{ - "files":{ - "private-key": "test_key.pem", - "ca-certificates": "test_ca.pem", - "certificates": "test_cert.pem" - } -} diff --git a/vespalib/src/tests/net/tls/transport_options/CMakeLists.txt b/vespalib/src/tests/net/tls/transport_options/CMakeLists.txt index 3623912bb42..2013879569f 100644 --- a/vespalib/src/tests/net/tls/transport_options/CMakeLists.txt +++ b/vespalib/src/tests/net/tls/transport_options/CMakeLists.txt @@ -4,6 +4,7 @@ vespa_add_executable(vespalib_net_tls_transport_options_test_app TEST transport_options_reading_test.cpp DEPENDS vespalib + GTest::gtest ) vespa_add_test(NAME vespalib_net_tls_transport_options_test_app COMMAND vespalib_net_tls_transport_options_test_app) diff --git a/vespalib/src/tests/net/tls/transport_options/ok_config.json b/vespalib/src/tests/net/tls/transport_options/ok_config.json deleted file mode 100644 index dd2591661dc..00000000000 --- a/vespalib/src/tests/net/tls/transport_options/ok_config.json +++ /dev/null @@ -1,7 +0,0 @@ -{ - "files":{ - "private-key": "dummy_privkey.txt", - "ca-certificates": "dummy_ca_certs.txt", - "certificates": "dummy_certs.txt" - } -} diff --git a/vespalib/src/tests/net/tls/transport_options/transport_options_reading_test.cpp b/vespalib/src/tests/net/tls/transport_options/transport_options_reading_test.cpp index ef0fdac0495..ea499607685 100644 --- a/vespalib/src/tests/net/tls/transport_options/transport_options_reading_test.cpp +++ b/vespalib/src/tests/net/tls/transport_options/transport_options_reading_test.cpp @@ -1,113 +1,245 @@ // Copyright Vespa.ai. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. + +#include <vespa/vespalib/gtest/gtest.h> #include <vespa/vespalib/io/fileutil.h> #include <vespa/vespalib/net/tls/transport_security_options.h> #include <vespa/vespalib/net/tls/transport_security_options_reading.h> #include <vespa/vespalib/test/peer_policy_utils.h> -#include <vespa/vespalib/testkit/test_kit.h> +#include <vespa/vespalib/testkit/test_path.h> #include <vespa/vespalib/util/exceptions.h> +#include <gmock/gmock.h> +#include <filesystem> +#include <fstream> +#include <optional> +#include <sstream> using namespace vespalib; using namespace vespalib::net::tls; -TEST("can load TLS credentials via config file") { +namespace { + +class ConfigWriter { + std::optional<std::string> _private_key; + std::optional<std::string> _ca_certificates; + std::optional<std::string> _certificates; + std::optional<std::string> _accepted_ciphers; + std::optional<std::string> _authorized_peers; + std::optional<std::string> _disable_hostname_validation; + std::optional<std::string> _flipper_the_dolphin; +public: + ConfigWriter(); + ConfigWriter(ConfigWriter&&); + ~ConfigWriter(); + ConfigWriter private_key(std::optional<std::string> value) && { _private_key = value; return std::move(*this); } + ConfigWriter ca_certificates(std::optional<std::string> value) && { _ca_certificates = value; return std::move(*this); } + ConfigWriter certificates(std::optional<std::string> value) && { _certificates = value; return std::move(*this); } + ConfigWriter accepted_ciphers(std::optional<std::string> value) && { _accepted_ciphers = value; return std::move(*this); } + ConfigWriter authorized_peers(std::optional<std::string> value) && { _authorized_peers = value; return std::move(*this); } + ConfigWriter disable_hostname_validation(std::optional<std::string> value) && { _disable_hostname_validation = value; return std::move(*this); } + ConfigWriter flipper_the_dolphin(std::optional<std::string> value) && { _flipper_the_dolphin = value; return std::move(*this); } + void write(std::ostream& os); + std::string write(); +}; + +ConfigWriter::ConfigWriter() + : _private_key(TEST_PATH("dummy_privkey.txt")), + _ca_certificates(TEST_PATH("dummy_ca_certs.txt")), + _certificates(TEST_PATH("dummy_certs.txt")), + _accepted_ciphers(), + _authorized_peers(), + _disable_hostname_validation(), + _flipper_the_dolphin() +{ +} + +ConfigWriter::ConfigWriter(ConfigWriter&&) = default; + +ConfigWriter::~ConfigWriter() = default; + +void +ConfigWriter::write(std::ostream& os) +{ + os << "{\n" << + R"( "files": {)"; + bool had_files_entry = false; + if (_private_key.has_value()) { + os << "\n" << R"( "private-key": ")" << _private_key.value() << R"(")"; + had_files_entry = true; + } + if (_ca_certificates.has_value()) { + if (had_files_entry) { + os << ","; + } + os << "\n" << R"( "ca-certificates": ")" << _ca_certificates.value() << R"(")"; + had_files_entry = true; + } + if (_certificates.has_value()) { + if (had_files_entry) { + os << ","; + } + os << "\n" << R"( "certificates": ")" << _certificates.value() << R"(")"; + had_files_entry = true; + } + os << "\n }"; + if (_accepted_ciphers.has_value()) { + os << ",\n" << R"( "accepted-ciphers" : )" << _accepted_ciphers.value(); + } + if (_authorized_peers.has_value()) { + os << ",\n" << R"( "authorized-peers": )" << _authorized_peers.value(); + } + if (_disable_hostname_validation.has_value()) { + os << ",\n" << R"( "disable-hostname-validation": )" << _disable_hostname_validation.value(); + } + if (_flipper_the_dolphin.has_value()) { + os << ",\n" << R"( "flipper-the-dolphin": )" << _flipper_the_dolphin.value(); + } + os << "\n}" << std::endl; +} + +std::string +ConfigWriter::write() +{ + std::ostringstream os; + write(os); + return os.str(); +} + +} + +class TransportSecurityOptionsTest : public ::testing::Test { +protected: + TransportSecurityOptionsTest(); + ~TransportSecurityOptionsTest() override; + static void SetUpTestSuite(); + static void TearDownTestSuite(); +}; + +TransportSecurityOptionsTest::TransportSecurityOptionsTest() + : ::testing::Test() +{ +} + +TransportSecurityOptionsTest::~TransportSecurityOptionsTest() = default; + +void +TransportSecurityOptionsTest::SetUpTestSuite() +{ + std::ofstream ok_config("ok_config.json"); + ConfigWriter().write(ok_config); + ok_config.close(); +} + +void +TransportSecurityOptionsTest::TearDownTestSuite() +{ + std::filesystem::remove("ok_config.json"); +} + +TEST_F(TransportSecurityOptionsTest, can_load_tls_credentials_via_config_file) +{ auto opts = read_options_from_json_file("ok_config.json"); ASSERT_TRUE(opts.get() != nullptr); // Obviously we'd need to change this to actual PEM data if config reading started // actually verifying the _content_ of files, not just reading them. - EXPECT_EQUAL("My private key\n", opts->private_key_pem()); - EXPECT_EQUAL("My CA certificates\n", opts->ca_certs_pem()); - EXPECT_EQUAL("My certificate chain\n", opts->cert_chain_pem()); + EXPECT_EQ("My private key\n", opts->private_key_pem()); + EXPECT_EQ("My CA certificates\n", opts->ca_certs_pem()); + EXPECT_EQ("My certificate chain\n", opts->cert_chain_pem()); } -TEST("copying options without private key does, in fact, not include private key") { +TEST_F(TransportSecurityOptionsTest, copying_options_without_private_key_does_in_fact_not_include_private_key) +{ auto opts = read_options_from_json_file("ok_config.json"); auto cloned = opts->copy_without_private_key(); - EXPECT_EQUAL("", cloned.private_key_pem()); - EXPECT_EQUAL("My CA certificates\n", cloned.ca_certs_pem()); - EXPECT_EQUAL("My certificate chain\n", cloned.cert_chain_pem()); + EXPECT_EQ("", cloned.private_key_pem()); + EXPECT_EQ("My CA certificates\n", cloned.ca_certs_pem()); + EXPECT_EQ("My certificate chain\n", cloned.cert_chain_pem()); } -TEST("missing JSON file throws exception") { - EXPECT_EXCEPTION(read_options_from_json_file("missing_config.json"), IllegalArgumentException, - "TLS config file 'missing_config.json' could not be read"); +TEST_F(TransportSecurityOptionsTest, missing_json_file_throws_exception) +{ + EXPECT_THAT([]() { read_options_from_json_file("missing_config.json"); }, + testing::ThrowsMessage<IllegalArgumentException>(testing::HasSubstr("TLS config file 'missing_config.json' could not be read"))); } -TEST("bad JSON content throws exception") { +TEST_F(TransportSecurityOptionsTest, bad_json_content_throws_exception) +{ const char* bad_json = "hello world :D"; - EXPECT_EXCEPTION(read_options_from_json_string(bad_json), IllegalArgumentException, - "Provided TLS config file is not valid JSON"); + EXPECT_THAT([bad_json]() { read_options_from_json_string(bad_json); }, + testing::ThrowsMessage<IllegalArgumentException>(testing::HasSubstr("Provided TLS config file is not valid JSON"))); } -TEST("missing 'files' field throws exception") { +TEST_F(TransportSecurityOptionsTest, missing_files_field_throws_exception) +{ const char* incomplete_json = R"({})"; - EXPECT_EXCEPTION(read_options_from_json_string(incomplete_json), IllegalArgumentException, - "TLS config root field 'files' is missing or empty"); + EXPECT_THAT([incomplete_json]() { read_options_from_json_string(incomplete_json); }, + testing::ThrowsMessage<IllegalArgumentException>(testing::HasSubstr("TLS config root field 'files' is missing or empty"))); } -TEST("missing 'private-key' field throws exception") { - const char* incomplete_json = R"({"files":{"certificates":"dummy_certs.txt","ca-certificates":"dummy_ca_certs.txt"}})"; - EXPECT_EXCEPTION(read_options_from_json_string(incomplete_json), IllegalArgumentException, - "TLS config field 'private-key' has not been set"); +TEST_F(TransportSecurityOptionsTest, missing_private_key_field_throws_exception) +{ + auto incomplete_json = ConfigWriter().private_key(std::nullopt).write(); + EXPECT_THAT([incomplete_json]() { read_options_from_json_string(incomplete_json); }, + testing::ThrowsMessage<IllegalArgumentException>(testing::HasSubstr("TLS config field 'private-key' has not been set"))); } -TEST("missing 'certificates' field throws exception") { - const char* incomplete_json = R"({"files":{"private-key":"dummy_privkey.txt","ca-certificates":"dummy_ca_certs.txt"}})"; - EXPECT_EXCEPTION(read_options_from_json_string(incomplete_json), IllegalArgumentException, - "TLS config field 'certificates' has not been set"); +TEST_F(TransportSecurityOptionsTest, missing_certificates_field_throws_exception) +{ + auto incomplete_json = ConfigWriter().certificates(std::nullopt).write(); + EXPECT_THAT([incomplete_json]() { read_options_from_json_string(incomplete_json); }, + testing::ThrowsMessage<IllegalArgumentException>(testing::HasSubstr("TLS config field 'certificates' has not been set"))); } -TEST("missing 'ca-certificates' field throws exception") { - const char* incomplete_json = R"({"files":{"private-key":"dummy_privkey.txt","certificates":"dummy_certs.txt"}})"; - EXPECT_EXCEPTION(read_options_from_json_string(incomplete_json), IllegalArgumentException, - "TLS config field 'ca-certificates' has not been set"); +TEST_F(TransportSecurityOptionsTest, missing_ca_certificates_field_throws_exception) +{ + auto incomplete_json = ConfigWriter().ca_certificates(std::nullopt).write(); + EXPECT_THAT([incomplete_json]() { read_options_from_json_string(incomplete_json); }, + testing::ThrowsMessage<IllegalArgumentException>(testing::HasSubstr("TLS config field 'ca-certificates' has not been set"))); } -TEST("missing file referenced by field throws exception") { - const char* incomplete_json = R"({"files":{"private-key":"missing_privkey.txt", - "certificates":"dummy_certs.txt", - "ca-certificates":"dummy_ca_certs.txt"}})"; - EXPECT_EXCEPTION(read_options_from_json_string(incomplete_json), IllegalArgumentException, - "File 'missing_privkey.txt' referenced by TLS config does not exist"); +TEST_F(TransportSecurityOptionsTest, missing_file_referenced_by_field_throws_exception) +{ + auto incomplete_json = ConfigWriter().private_key("missing_privkey.txt").write(); + EXPECT_THAT([incomplete_json]() { read_options_from_json_string(incomplete_json); }, + testing::ThrowsMessage<IllegalArgumentException>(testing::HasSubstr("File 'missing_privkey.txt' referenced by TLS config does not exist"))); } vespalib::string json_with_policies(const vespalib::string& policies) { - const char* fmt = R"({"files":{"private-key":"dummy_privkey.txt", - "certificates":"dummy_certs.txt", - "ca-certificates":"dummy_ca_certs.txt"}, - "authorized-peers":[%s]})"; - return vespalib::make_string(fmt, policies.c_str()); + return ConfigWriter().authorized_peers(std::string("[") + policies + "]").write(); } -TransportSecurityOptions parse_policies(const vespalib::string& policies) { +TransportSecurityOptions parse_policies(const vespalib::string& policies) +{ return *read_options_from_json_string(json_with_policies(policies)); } -TEST("config file without authorized-peers accepts all pre-verified certificates") { - const char* json = R"({"files":{"private-key":"dummy_privkey.txt", - "certificates":"dummy_certs.txt", - "ca-certificates":"dummy_ca_certs.txt"}})"; +TEST_F(TransportSecurityOptionsTest, config_file_without_authorized_peers_accepts_all_pre_verified_certificates) +{ + auto json = ConfigWriter().write(); EXPECT_TRUE(read_options_from_json_string(json)->authorized_peers().allows_all_authenticated()); } // Instead of contemplating what the semantics of an empty allow list should be, // we do the easy way out and just say it's not allowed in the first place. -TEST("empty policy array throws exception") { - EXPECT_EXCEPTION(parse_policies(""), vespalib::IllegalArgumentException, - "\"authorized-peers\" must either be not present (allows " - "all peers with valid certificates) or a non-empty array"); +TEST_F(TransportSecurityOptionsTest, empty_policy_array_throws_exception) +{ + EXPECT_THAT([]() { parse_policies(""); }, + testing::ThrowsMessage<IllegalArgumentException>(testing::HasSubstr("\"authorized-peers\" must either be not present (allows " + "all peers with valid certificates) or a non-empty array"))); } -TEST("can parse single peer policy with single requirement") { +TEST_F(TransportSecurityOptionsTest, can_parse_single_peer_policy_with_single_requirement) +{ const char* json = R"({ "required-credentials":[ {"field": "SAN_DNS", "must-match": "hello.world"} ] })"; - EXPECT_EQUAL(authorized_peers({policy_with({required_san_dns("hello.world")})}), - parse_policies(json).authorized_peers()); + EXPECT_EQ(authorized_peers({policy_with({required_san_dns("hello.world")})}), + parse_policies(json).authorized_peers()); } -TEST("can parse single peer policy with multiple requirements") { +TEST_F(TransportSecurityOptionsTest, can_parse_single_peer_policy_with_multiple_requirements) +{ const char* json = R"({ "required-credentials":[ {"field": "SAN_DNS", "must-match": "hello.world"}, @@ -115,57 +247,56 @@ TEST("can parse single peer policy with multiple requirements") { {"field": "CN", "must-match": "goodbye.moon"} ] })"; - EXPECT_EQUAL(authorized_peers({policy_with({required_san_dns("hello.world"), - required_san_uri("foo://bar/baz"), - required_cn("goodbye.moon")})}), - parse_policies(json).authorized_peers()); + EXPECT_EQ(authorized_peers({policy_with({required_san_dns("hello.world"), + required_san_uri("foo://bar/baz"), + required_cn("goodbye.moon")})}), + parse_policies(json).authorized_peers()); } -TEST("unknown field type throws exception") { +TEST_F(TransportSecurityOptionsTest, unknown_field_type_throws_exception) +{ const char* json = R"({ "required-credentials":[ {"field": "winnie the pooh", "must-match": "piglet"} ] })"; - EXPECT_EXCEPTION(parse_policies(json), vespalib::IllegalArgumentException, - "Unsupported credential field type: 'winnie the pooh'. Supported are: CN, SAN_DNS"); + EXPECT_THAT([json]() { parse_policies(json); }, + testing::ThrowsMessage<IllegalArgumentException>(testing::HasSubstr("Unsupported credential field type: 'winnie the pooh'. Supported are: CN, SAN_DNS"))); } -TEST("empty required-credentials array throws exception") { +TEST_F(TransportSecurityOptionsTest, empty_required_credentials_array_throws_exception) +{ const char* json = R"({ "required-credentials":[] })"; - EXPECT_EXCEPTION(parse_policies(json), vespalib::IllegalArgumentException, - "\"required-credentials\" array can't be empty (would allow all peers)"); + EXPECT_THAT([json]() { parse_policies(json); }, + testing::ThrowsMessage<IllegalArgumentException>(testing::HasSubstr("\"required-credentials\" array can't be empty (would allow all peers)"))); } -TEST("accepted cipher list is empty if not specified") { - const char* json = R"({"files":{"private-key":"dummy_privkey.txt", - "certificates":"dummy_certs.txt", - "ca-certificates":"dummy_ca_certs.txt"}})"; +TEST_F(TransportSecurityOptionsTest, accepted_cipher_list_is_empty_if_not_specified) +{ + auto json = ConfigWriter().write(); EXPECT_TRUE(read_options_from_json_string(json)->accepted_ciphers().empty()); } -TEST("accepted cipher list is populated if specified") { - const char* json = R"({"files":{"private-key":"dummy_privkey.txt", - "certificates":"dummy_certs.txt", - "ca-certificates":"dummy_ca_certs.txt"}, - "accepted-ciphers":["foo", "bar"]})"; +TEST_F(TransportSecurityOptionsTest, accepted_cipher_list_is_populated_if_specified) +{ + auto json = ConfigWriter().accepted_ciphers(R"(["foo", "bar"])").write(); auto ciphers = read_options_from_json_string(json)->accepted_ciphers(); - ASSERT_EQUAL(2u, ciphers.size()); - EXPECT_EQUAL("foo", ciphers[0]); - EXPECT_EQUAL("bar", ciphers[1]); + ASSERT_EQ(2u, ciphers.size()); + EXPECT_EQ("foo", ciphers[0]); + EXPECT_EQ("bar", ciphers[1]); } // FIXME this is temporary until we know enabling it by default won't break the world! -TEST("hostname validation is DISABLED by default when creating options from config file") { - const char* json = R"({"files":{"private-key":"dummy_privkey.txt", - "certificates":"dummy_certs.txt", - "ca-certificates":"dummy_ca_certs.txt"}})"; +TEST_F(TransportSecurityOptionsTest, hostname_validation_is_DISABLED_by_default_when_creating_options_from_config_file) +{ + auto json = ConfigWriter().write(); EXPECT_TRUE(read_options_from_json_string(json)->disable_hostname_validation()); } -TEST("TransportSecurityOptions builder does not disable hostname validation by default") { +TEST_F(TransportSecurityOptionsTest, transport_security_options_builder_does_not_disable_hostname_validation_by_default) +{ auto ts_builder = vespalib::net::tls::TransportSecurityOptions::Params(). ca_certs_pem("foo"). cert_chain_pem("bar"). @@ -174,67 +305,65 @@ TEST("TransportSecurityOptions builder does not disable hostname validation by d EXPECT_FALSE(ts_opts.disable_hostname_validation()); } -TEST("hostname validation can be explicitly disabled") { - const char* json = R"({"files":{"private-key":"dummy_privkey.txt", - "certificates":"dummy_certs.txt", - "ca-certificates":"dummy_ca_certs.txt"}, - "disable-hostname-validation": true})"; +TEST_F(TransportSecurityOptionsTest, hostname_validation_can_be_explicitly_disabled) +{ + auto json = ConfigWriter().disable_hostname_validation("true").write(); EXPECT_TRUE(read_options_from_json_string(json)->disable_hostname_validation()); } -TEST("hostname validation can be explicitly enabled") { - const char* json = R"({"files":{"private-key":"dummy_privkey.txt", - "certificates":"dummy_certs.txt", - "ca-certificates":"dummy_ca_certs.txt"}, - "disable-hostname-validation": false})"; +TEST_F(TransportSecurityOptionsTest, hostname_validation_can_be_explicitly_enabled) +{ + auto json = ConfigWriter().disable_hostname_validation("false").write(); EXPECT_FALSE(read_options_from_json_string(json)->disable_hostname_validation()); } -TEST("unknown fields are ignored at parse-time") { - const char* json = R"({"files":{"private-key":"dummy_privkey.txt", - "certificates":"dummy_certs.txt", - "ca-certificates":"dummy_ca_certs.txt"}, - "flipper-the-dolphin": "*weird dolphin noises*"})"; +TEST_F(TransportSecurityOptionsTest, unknown_fields_are_ignored_at_parse_time) +{ + auto json = ConfigWriter().flipper_the_dolphin(R"("*weird dolphin noises*")").write(); EXPECT_TRUE(read_options_from_json_string(json).get() != nullptr); // And no exception thrown. } -TEST("policy without explicit capabilities implicitly get all capabilities") { +TEST_F(TransportSecurityOptionsTest, policy_without_explicit_capabilities_implicitly_get_all_capabilities) +{ const char* json = R"({ "required-credentials":[ {"field": "SAN_DNS", "must-match": "hello.world"} ] })"; - EXPECT_EQUAL(authorized_peers({policy_with({required_san_dns("hello.world")}, - CapabilitySet::make_with_all_capabilities())}), - parse_policies(json).authorized_peers()); + EXPECT_EQ(authorized_peers({policy_with({required_san_dns("hello.world")}, + CapabilitySet::make_with_all_capabilities())}), + parse_policies(json).authorized_peers()); } -TEST("specifying a capability set adds all its underlying capabilities") { +TEST_F(TransportSecurityOptionsTest, specifying_a_capability_set_adds_all_its_underlying_capabilities) +{ const char* json = R"({ "required-credentials":[ {"field": "SAN_DNS", "must-match": "*.cool-content-clusters.example" } ], "capabilities": ["vespa.content_node"] })"; - EXPECT_EQUAL(authorized_peers({policy_with({required_san_dns("*.cool-content-clusters.example")}, - CapabilitySet::content_node())}), - parse_policies(json).authorized_peers()); + EXPECT_EQ(authorized_peers({policy_with({required_san_dns("*.cool-content-clusters.example")}, + CapabilitySet::content_node())}), + parse_policies(json).authorized_peers()); } -TEST("can specify single leaf capabilities") { +TEST_F(TransportSecurityOptionsTest, can_specify_single_leaf_capabilities) +{ const char* json = R"({ "required-credentials":[ {"field": "SAN_DNS", "must-match": "*.cool-content-clusters.example" } ], "capabilities": ["vespa.content.metrics_api", "vespa.slobrok.api"] })"; - EXPECT_EQUAL(authorized_peers({policy_with({required_san_dns("*.cool-content-clusters.example")}, - CapabilitySet::of({Capability::content_metrics_api(), - Capability::slobrok_api()}))}), - parse_policies(json).authorized_peers()); + EXPECT_EQ(authorized_peers({policy_with({required_san_dns("*.cool-content-clusters.example")}, + CapabilitySet::of({Capability::content_metrics_api(), + Capability::slobrok_api()}))}), + parse_policies(json).authorized_peers()); } -TEST("specifying multiple capability sets adds union of underlying capabilities") { +TEST_F(TransportSecurityOptionsTest, specifying_multiple_capability_sets_adds_union_of_underlying_capabilities) +{ const char* json = R"({ "required-credentials":[ {"field": "SAN_DNS", "must-match": "*.cool-content-clusters.example" } @@ -244,23 +373,22 @@ TEST("specifying multiple capability sets adds union of underlying capabilities" CapabilitySet caps; caps.add_all(CapabilitySet::content_node()); caps.add_all(CapabilitySet::container_node()); - EXPECT_EQUAL(authorized_peers({policy_with({required_san_dns("*.cool-content-clusters.example")}, caps)}), - parse_policies(json).authorized_peers()); + EXPECT_EQ(authorized_peers({policy_with({required_san_dns("*.cool-content-clusters.example")}, caps)}), + parse_policies(json).authorized_peers()); } -TEST("empty capabilities array is not allowed") { +TEST_F(TransportSecurityOptionsTest, empty_capabilities_array_is_not_allowed) { const char* json = R"({ "required-credentials":[ {"field": "SAN_DNS", "must-match": "*.cool-content-clusters.example" } ], "capabilities": [] })"; - EXPECT_EXCEPTION(parse_policies(json), vespalib::IllegalArgumentException, - "\"capabilities\" array must either be not present (implies " - "all capabilities) or contain at least one capability name"); + EXPECT_THAT([json]() { parse_policies(json); }, + testing::ThrowsMessage<IllegalArgumentException>(testing::HasSubstr("\"capabilities\" array must either be not present (implies " + "all capabilities) or contain at least one capability name"))); } // TODO test parsing of multiple policies -TEST_MAIN() { TEST_RUN_ALL(); } - +GTEST_MAIN_RUN_ALL_TESTS() diff --git a/vespalib/src/tests/polymorphicarray/polymorphicarray_test.cpp b/vespalib/src/tests/polymorphicarray/polymorphicarray_test.cpp index 212ea417524..62b7dc5f179 100644 --- a/vespalib/src/tests/polymorphicarray/polymorphicarray_test.cpp +++ b/vespalib/src/tests/polymorphicarray/polymorphicarray_test.cpp @@ -2,6 +2,7 @@ #include <vespa/vespalib/testkit/test_kit.h> #include <vespa/vespalib/util/polymorphicarrays.h> +#include <cassert> using namespace vespalib; diff --git a/vespalib/src/tests/rendezvous/rendezvous_test.cpp b/vespalib/src/tests/rendezvous/rendezvous_test.cpp index d2e2ac2fbab..13c4f968db1 100644 --- a/vespalib/src/tests/rendezvous/rendezvous_test.cpp +++ b/vespalib/src/tests/rendezvous/rendezvous_test.cpp @@ -2,6 +2,7 @@ #include <vespa/vespalib/testkit/test_kit.h> #include <vespa/vespalib/util/rendezvous.h> #include <vespa/vespalib/util/time.h> +#include <vespa/vespalib/util/count_down_latch.h> #include <utility> #include <thread> diff --git a/vespalib/src/tests/shared_operation_throttler/shared_operation_throttler_test.cpp b/vespalib/src/tests/shared_operation_throttler/shared_operation_throttler_test.cpp index 0f1c6d3a083..18613c0bc79 100644 --- a/vespalib/src/tests/shared_operation_throttler/shared_operation_throttler_test.cpp +++ b/vespalib/src/tests/shared_operation_throttler/shared_operation_throttler_test.cpp @@ -3,6 +3,7 @@ #include <vespa/vespalib/testkit/test_kit.h> #include <vespa/vespalib/util/barrier.h> #include <thread> +#include <cassert> using vespalib::steady_clock; diff --git a/vespalib/src/tests/slime/json_slime_benchmark.cpp b/vespalib/src/tests/slime/json_slime_benchmark.cpp index 78000a5a25d..df9492ba46c 100644 --- a/vespalib/src/tests/slime/json_slime_benchmark.cpp +++ b/vespalib/src/tests/slime/json_slime_benchmark.cpp @@ -4,6 +4,7 @@ #include <vespa/vespalib/testkit/test_kit.h> #include <iostream> #include <fstream> +#include <cassert> using namespace vespalib::slime::convenience; diff --git a/vespalib/src/tests/slime/summary-feature-benchmark/summary-feature-benchmark.cpp b/vespalib/src/tests/slime/summary-feature-benchmark/summary-feature-benchmark.cpp index 87ecb42efa8..065984e5ed9 100644 --- a/vespalib/src/tests/slime/summary-feature-benchmark/summary-feature-benchmark.cpp +++ b/vespalib/src/tests/slime/summary-feature-benchmark/summary-feature-benchmark.cpp @@ -3,6 +3,7 @@ #include <vespa/vespalib/util/size_literals.h> #include <vespa/vespalib/util/stringfmt.h> #include <vespa/vespalib/data/slime/slime.h> +#include <cassert> using namespace vespalib; using namespace vespalib::slime::convenience; diff --git a/vespalib/src/vespa/vespalib/test/CMakeLists.txt b/vespalib/src/vespa/vespalib/test/CMakeLists.txt index 4a43f9a92dd..b3b872f14c7 100644 --- a/vespalib/src/vespa/vespalib/test/CMakeLists.txt +++ b/vespalib/src/vespa/vespalib/test/CMakeLists.txt @@ -5,6 +5,7 @@ vespa_add_library(vespalib_vespalib_test OBJECT memory_allocator_observer.cpp nexus.cpp peer_policy_utils.cpp + test_data_base.cpp thread_meets.cpp time_tracer.cpp DEPENDS diff --git a/vespalib/src/vespa/vespalib/test/socket_options_verifier.h b/vespalib/src/vespa/vespalib/test/socket_options_verifier.h index 04b4dea414e..3831c03d629 100644 --- a/vespalib/src/vespa/vespalib/test/socket_options_verifier.h +++ b/vespalib/src/vespa/vespalib/test/socket_options_verifier.h @@ -2,7 +2,7 @@ #pragma once -#include <vespa/vespalib/testkit/test_kit.h> +#include <vespa/vespalib/gtest/gtest.h> #include <fcntl.h> #include <unistd.h> #include <netinet/tcp.h> @@ -16,9 +16,9 @@ namespace { void verify_bool_opt(int fd, int level, int name, bool expect) { int data = 0; socklen_t size = sizeof(data); - EXPECT_EQUAL(getsockopt(fd, level, name, &data, &size), 0); - EXPECT_EQUAL(size, sizeof(data)); - EXPECT_EQUAL(data != 0, expect); + EXPECT_EQ(getsockopt(fd, level, name, &data, &size), 0); + EXPECT_EQ(size, sizeof(data)); + EXPECT_EQ(data != 0, expect); } } // namespace vespalib::test::<unnamed> @@ -31,31 +31,35 @@ struct SocketOptionsVerifier { SocketOptionsVerifier(int fd_in) : fd(fd_in) {} void verify_blocking(bool value) { int flags = fcntl(fd, F_GETFL, NULL); - EXPECT_NOT_EQUAL(flags, -1); - EXPECT_EQUAL(((flags & O_NONBLOCK) == 0), value); + EXPECT_NE(flags, -1); + EXPECT_EQ(((flags & O_NONBLOCK) == 0), value); } void verify_nodelay(bool value) { - TEST_DO(verify_bool_opt(fd, IPPROTO_TCP, TCP_NODELAY, value)); + SCOPED_TRACE("verify nodelay"); + verify_bool_opt(fd, IPPROTO_TCP, TCP_NODELAY, value); } void verify_reuse_addr(bool value) { - TEST_DO(verify_bool_opt(fd, SOL_SOCKET, SO_REUSEADDR, value)); + SCOPED_TRACE("verify reuse addr"); + verify_bool_opt(fd, SOL_SOCKET, SO_REUSEADDR, value); } void verify_ipv6_only(bool value) { - TEST_DO(verify_bool_opt(fd, IPPROTO_IPV6, IPV6_V6ONLY, value)); + SCOPED_TRACE("verify ipv6 only"); + verify_bool_opt(fd, IPPROTO_IPV6, IPV6_V6ONLY, value); } void verify_keepalive(bool value) { - TEST_DO(verify_bool_opt(fd, SOL_SOCKET, SO_KEEPALIVE, value)); + SCOPED_TRACE("verify keepalive"); + verify_bool_opt(fd, SOL_SOCKET, SO_KEEPALIVE, value); } void verify_linger(bool enable, int value) { struct linger data; socklen_t size = sizeof(data); memset(&data, 0, sizeof(data)); - EXPECT_EQUAL(getsockopt(fd, SOL_SOCKET, SO_LINGER, &data, &size), 0); - EXPECT_EQUAL(size, sizeof(data)); - EXPECT_EQUAL(enable, data.l_onoff); + EXPECT_EQ(getsockopt(fd, SOL_SOCKET, SO_LINGER, &data, &size), 0); + EXPECT_EQ(size, sizeof(data)); + EXPECT_EQ(enable, data.l_onoff); if (enable) { - EXPECT_EQUAL(value, data.l_linger); + EXPECT_EQ(value, data.l_linger); } } }; diff --git a/vespalib/src/vespa/vespalib/test/test_data.h b/vespalib/src/vespa/vespalib/test/test_data.h new file mode 100644 index 00000000000..1a5c24616b0 --- /dev/null +++ b/vespalib/src/vespa/vespalib/test/test_data.h @@ -0,0 +1,60 @@ +// Copyright Vespa.ai. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. + +#pragma once + +#include "test_data_base.h" +#include <vespa/vespalib/gtest/gtest.h> +#include <filesystem> + +namespace vespalib::test { + +// Utility class for accessing test data used by unit tests. +template <class Derived> +class TestData : public TestDataBase { +protected: + static std::string _source_testdata; + static std::string _build_testdata; +public: + static void setup_test_data(const std::string& source_testdata_in, const std::string& build_testdata_in); + static void tear_down_test_data(); + static const std::string& source_testdata() noexcept { return _source_testdata; } + static const std::string& build_testdata() noexcept { return _build_testdata; } + void remove_unchanged_build_testdata_file_or_fail(const nbostream& buf, const std::string& file_name); +}; + +template <class Derived> +std::string TestData<Derived>::_source_testdata; + +template <class Derived> +std::string TestData<Derived>::_build_testdata; + +template <class Derived> +void +TestData<Derived>::setup_test_data(const std::string& source_testdata_in, const std::string& build_testdata_in) +{ + _source_testdata = source_testdata_in; + _build_testdata = build_testdata_in; + std::filesystem::create_directory(build_testdata()); +} + +template <class Derived> +void +TestData<Derived>::tear_down_test_data() +{ + std::filesystem::remove(build_testdata()); +} + +template <class Derived> +void +TestData<Derived>::remove_unchanged_build_testdata_file_or_fail(const nbostream& buf, const std::string& file_name) +{ + auto act_path = build_testdata() + "/" + file_name; + auto exp_path = source_testdata() + "/" + file_name; + ASSERT_TRUE(std::filesystem::exists(exp_path)) << "Missing expected contents file " << exp_path; + auto exp_buf = read_buffer_from_file(exp_path); + ASSERT_TRUE(equiv_buffers(exp_buf, buf)) << "Files " << exp_path << " and " << act_path << + " have diferent contents"; + std::filesystem::remove(act_path); +} + +} diff --git a/vespalib/src/vespa/vespalib/test/test_data_base.cpp b/vespalib/src/vespa/vespalib/test/test_data_base.cpp new file mode 100644 index 00000000000..12bfca50d5e --- /dev/null +++ b/vespalib/src/vespa/vespalib/test/test_data_base.cpp @@ -0,0 +1,45 @@ +// Copyright Vespa.ai. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. + +#include "test_data.h" +#include <vespa/vespalib/objects/nbostream.h> +#include <cassert> +#include <cstring> +#include <fstream> + +namespace vespalib::test { + +bool +TestDataBase::equiv_buffers(const nbostream& lhs, const nbostream& rhs) +{ + return lhs.size() == rhs.size() && memcmp(lhs.data(), rhs.data(), lhs.size()) == 0; +} + +nbostream +TestDataBase::read_buffer_from_file(const std::string& path) +{ + auto file = std::ifstream(path, std::ios::in | std::ios::binary | std::ios::ate); + auto size = file.tellg(); + file.seekg(0); + vespalib::alloc::Alloc buf = vespalib::alloc::Alloc::alloc(size); + file.read(static_cast<char *>(buf.get()), size); + assert(file.good()); + file.close(); + return nbostream(std::move(buf), size); +} + +void +TestDataBase::write_buffer_to_file(const nbostream& buf, const std::string& path) +{ + write_buffer_to_file(std::string_view{buf.data(), buf.size()}, path); +} + +void +TestDataBase::write_buffer_to_file(std::string_view buf, const std::string& path) +{ + auto file = std::ofstream(path, std::ios::out | std::ios::binary | std::ios::trunc); + file.write(buf.data(), buf.size()); + assert(file.good()); + file.close(); +} + +} diff --git a/vespalib/src/vespa/vespalib/test/test_data_base.h b/vespalib/src/vespa/vespalib/test/test_data_base.h new file mode 100644 index 00000000000..65ea4533f81 --- /dev/null +++ b/vespalib/src/vespa/vespalib/test/test_data_base.h @@ -0,0 +1,20 @@ +// Copyright Vespa.ai. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. + +#pragma once + +#include <string_view> + +namespace vespalib { class nbostream; } + +namespace vespalib::test { + +// Utility base class for accessing test data used by unit tests. +class TestDataBase { +public: + static bool equiv_buffers(const nbostream& lhs, const nbostream& rhs); + static nbostream read_buffer_from_file(const std::string& path); + static void write_buffer_to_file(const nbostream& buf, const std::string& path); + static void write_buffer_to_file(std::string_view buf, const std::string& path); +}; + +} diff --git a/vespalib/src/vespa/vespalib/testkit/test_hook.cpp b/vespalib/src/vespa/vespalib/testkit/test_hook.cpp index 4f7c7cf53bc..b14b575ae93 100644 --- a/vespalib/src/vespa/vespalib/testkit/test_hook.cpp +++ b/vespalib/src/vespa/vespalib/testkit/test_hook.cpp @@ -1,8 +1,11 @@ // Copyright Vespa.ai. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. #include "test_hook.h" +#include <vespa/vespalib/util/count_down_latch.h> +#include <vespa/vespalib/util/barrier.h> +#include <vespa/vespalib/util/thread.h> #include <vespa/vespalib/util/stringfmt.h> -#include <vespa/vespalib/util/size_literals.h> +#include <cassert> #include <regex> namespace vespalib { @@ -29,7 +32,7 @@ TestThreadWrapper::threadEntry() master.check(false, __FILE__, __LINE__, "test threw stuff", false); } _barrier.destroy(); - master.setThreadBarrier(0); + master.setThreadBarrier(nullptr); bool fail = (master.getThreadFailCnt() > preThreadFailCnt); master.setThreadUnwind(false); master.setThreadIgnore(false); @@ -39,30 +42,64 @@ TestThreadWrapper::threadEntry() master.setThreadName(oldThreadName.c_str()); } -TestHook *TestHook::_head = 0; -TestHook *TestHook::_tail = 0; +TestHook *TestHook::_head = nullptr; +TestHook *TestHook::_tail = nullptr; + +TestHook::~TestHook() = default; TestHook::TestHook(const std::string &file, const std::string &name, bool ignore) - : _next(0), + : _next(nullptr), _name(name), _tag(make_string("%s:%s", file.c_str(), name.c_str())), _ignore(ignore) { - if (_head == 0) { - assert(_tail == 0); + if (_head == nullptr) { + assert(_tail == nullptr); _head = this; _tail = this; } else { - assert(_tail != 0); - assert(_tail->_next == 0); + assert(_tail != nullptr); + assert(_tail->_next == nullptr); _tail->_next = this; _tail = this; } } +bool TestHook::runMyTest(const FixtureFactory & fixture_factory, size_t num_threads) { + assert(num_threads > 0); + using ThreadUP = std::unique_ptr<TestThreadWrapper>; + using FixtureUP = std::unique_ptr<TestFixtureWrapper>; + std::vector<TestMaster::TraceItem> traceStack = TestMaster::master.getThreadTraceStack(); + CountDownLatch latch(num_threads); + Barrier barrier(num_threads); + std::vector<FixtureUP> fixtures; + std::vector<ThreadUP> threads; + ThreadPool pool; + threads.reserve(num_threads); + fixtures.reserve(num_threads); + for (size_t i = 0; i < num_threads; ++i) { + FixtureUP fixture_up = fixture_factory(); + fixture_up->thread_id = i; + fixture_up->num_threads = num_threads; + threads.emplace_back(new TestThreadWrapper(_ignore, latch, barrier, traceStack, *fixture_up)); + fixtures.push_back(std::move(fixture_up)); + } + for (size_t i = 1; i < num_threads; ++i) { + pool.start([&target = *threads[i]](){ target.threadEntry(); }); + } + threads[0]->threadEntry(); + latch.await(); + pool.join(); + bool result = true; + for (size_t i = 0; i < num_threads; ++i) { + result = result && threads[i]->getResult(); + } + return result; +} + const char *lookup_subset_pattern(const std::string &name) { const char *pattern = getenv("TEST_SUBSET"); - if (pattern != 0) { + if (pattern != nullptr) { fprintf(stderr, "%s: info: only running tests matching '%s'\n", name.c_str(), pattern); } else { @@ -80,7 +117,7 @@ TestHook::runAll() size_t testsFailed = 0; size_t testsIgnored = 0; size_t testsSkipped = 0; - for (TestHook *test = _head; test != 0; test = test->_next) { + for (TestHook *test = _head; test != nullptr; test = test->_next) { if (std::regex_search(test->_tag, pattern)) { bool ignored = test->_ignore; bool failed = !test->run(); diff --git a/vespalib/src/vespa/vespalib/testkit/test_hook.h b/vespalib/src/vespa/vespalib/testkit/test_hook.h index fd9d743ff19..6887ae157d1 100644 --- a/vespalib/src/vespa/vespalib/testkit/test_hook.h +++ b/vespalib/src/vespa/vespalib/testkit/test_hook.h @@ -2,28 +2,24 @@ #pragma once -#include <vespa/vespalib/util/count_down_latch.h> -#include <vespa/vespalib/util/barrier.h> -#include <vespa/vespalib/util/thread.h> -#include <thread> -#include <string> -#include <vector> -#include <cassert> #include "test_master.h" +#include <functional> namespace vespalib { +class CountDownLatch; + struct TestThreadEntry { virtual void threadEntry() = 0; - virtual ~TestThreadEntry() {} + virtual ~TestThreadEntry() = default; }; struct TestFixtureWrapper { size_t thread_id; size_t num_threads; - TestFixtureWrapper() : thread_id(0), num_threads(1) {} + TestFixtureWrapper() noexcept: thread_id(0), num_threads(1) {} virtual void test_entry_point() = 0; - virtual ~TestFixtureWrapper() {} + virtual ~TestFixtureWrapper() = default; }; class TestThreadWrapper : public TestThreadEntry @@ -39,13 +35,13 @@ private: public: TestThreadWrapper(bool ignore, CountDownLatch &l, Barrier &b, const std::vector<TestMaster::TraceItem> &traceStack, - TestFixtureWrapper &fixture) + TestFixtureWrapper &fixture) noexcept : _result(false), _ignore(ignore), _latch(l), _barrier(b), _traceStack(traceStack), _fixture(fixture) {} void threadEntry() override; - bool getResult() const { + bool getResult() const noexcept { return _result; } }; @@ -61,48 +57,21 @@ private: std::string _tag; bool _ignore; - TestHook(const TestHook &); - TestHook &operator=(const TestHook &); - + using FixtureFactory = std::function<std::unique_ptr<TestFixtureWrapper>()>; + bool runMyTest(const FixtureFactory &fixture_factory, size_t num_threads); protected: TestHook(const std::string &file, const std::string &name, bool ignore); - virtual ~TestHook() {} + virtual ~TestHook(); template <typename T> bool runTest(const T &fixture, size_t num_threads) { - assert(num_threads > 0); - using ThreadUP = std::unique_ptr<TestThreadWrapper>; - using FixtureUP = std::unique_ptr<T>; - std::vector<TestMaster::TraceItem> traceStack = TestMaster::master.getThreadTraceStack(); - CountDownLatch latch(num_threads); - Barrier barrier(num_threads); - std::vector<FixtureUP> fixtures; - std::vector<ThreadUP> threads; - ThreadPool pool; - threads.reserve(num_threads); - fixtures.reserve(num_threads); - for (size_t i = 0; i < num_threads; ++i) { - FixtureUP fixture_up(new T(fixture)); - fixture_up->thread_id = i; - fixture_up->num_threads = num_threads; - threads.emplace_back(new TestThreadWrapper(_ignore, latch, barrier, traceStack, *fixture_up)); - fixtures.push_back(std::move(fixture_up)); - } - for (size_t i = 1; i < num_threads; ++i) { - pool.start([&target = *threads[i]](){ target.threadEntry(); }); - } - threads[0]->threadEntry(); - latch.await(); - pool.join(); - bool result = true; - for (size_t i = 0; i < num_threads; ++i) { - result = result && threads[i]->getResult(); - } - return result; + return runMyTest([&fixture]() { return std::make_unique<T>(fixture); }, num_threads); } virtual bool run() = 0; public: + TestHook(const TestHook &) = delete; + TestHook &operator=(const TestHook &) = delete; static void runAll(); }; #endif diff --git a/vespalib/src/vespa/vespalib/testkit/test_master.cpp b/vespalib/src/vespa/vespalib/testkit/test_master.cpp index 482699d05de..4698d40ecc5 100644 --- a/vespalib/src/vespa/vespalib/testkit/test_master.cpp +++ b/vespalib/src/vespa/vespalib/testkit/test_master.cpp @@ -38,6 +38,12 @@ TestMaster::TraceItem::TraceItem(const TraceItem &) = default; TestMaster::TraceItem & TestMaster::TraceItem::operator=(const TraceItem &) = default; TestMaster::TraceItem::~TraceItem() = default; +TestMaster::ThreadState::~ThreadState() = default; +TestMaster::ThreadState::ThreadState(const std::string &n) + : name(n), passCnt(0), failCnt(0), preIgnoreFailCnt(0), + ignore(false), unwind(false), traceStack(), barrier(nullptr) +{} + TestMaster::ThreadState & TestMaster::threadState(const lock_guard &) { @@ -171,6 +177,8 @@ TestMaster::TestMaster() setThreadName("master"); } +TestMaster::~TestMaster() = default; + //----------------------------------------------------------------------------- void diff --git a/vespalib/src/vespa/vespalib/testkit/test_master.h b/vespalib/src/vespa/vespalib/testkit/test_master.h index aa4633dc8fd..232ef8009c9 100644 --- a/vespalib/src/vespa/vespalib/testkit/test_master.h +++ b/vespalib/src/vespa/vespalib/testkit/test_master.h @@ -17,10 +17,6 @@ class Barrier; **/ class TestMaster { -private: - TestMaster(const TestMaster &); - TestMaster &operator=(const TestMaster &); - public: struct Progress { size_t passCnt; @@ -47,17 +43,17 @@ public: private: struct ThreadState { std::string name; - bool unwind; size_t passCnt; size_t failCnt; - bool ignore; size_t preIgnoreFailCnt; + bool ignore; + bool unwind; std::vector<TraceItem> traceStack; Barrier *barrier; - ThreadState(const std::string &n) - : name(n), unwind(false), passCnt(0), - failCnt(0), ignore(false), preIgnoreFailCnt(0), traceStack(), - barrier(0) {} + ~ThreadState(); + ThreadState(const std::string &n); + ThreadState(ThreadState &&) noexcept = default; + ThreadState & operator=(ThreadState &&) noexcept = default; }; static __thread ThreadState *_threadState; @@ -66,11 +62,12 @@ private: size_t failCnt; FILE *lhsFile; FILE *rhsFile; - SharedState() : passCnt(0), failCnt(0), - lhsFile(0), rhsFile(0) {} + SharedState() noexcept + : passCnt(0), failCnt(0), + lhsFile(nullptr), rhsFile(nullptr) + {} }; -private: std::mutex _lock; std::string _name; SharedState _state; @@ -78,6 +75,7 @@ private: using lock_guard = std::lock_guard<std::mutex>; private: + TestMaster(); ThreadState &threadState(const lock_guard &); ThreadState &threadState(); void checkFailed(const lock_guard &, @@ -90,10 +88,11 @@ private: void importThreads(const lock_guard &); bool reportConclusion(const lock_guard &); -private: - TestMaster(); public: + ~TestMaster(); + TestMaster(const TestMaster &) = delete; + TestMaster &operator=(const TestMaster &) = delete; void init(const char *name); std::string getName(); void setThreadName(const char *name); diff --git a/vespamalloc/src/tests/test1/new_test.cpp b/vespamalloc/src/tests/test1/new_test.cpp index 33b26f857dd..74a45f9a9d0 100644 --- a/vespamalloc/src/tests/test1/new_test.cpp +++ b/vespamalloc/src/tests/test1/new_test.cpp @@ -6,6 +6,7 @@ #include <malloc.h> #include <dlfcn.h> #include <functional> +#include <cassert> LOG_SETUP("new_test"); diff --git a/vespamalloc/src/tests/thread/racemanythreads.cpp b/vespamalloc/src/tests/thread/racemanythreads.cpp index 3f153825844..d5ef2475e9a 100644 --- a/vespamalloc/src/tests/thread/racemanythreads.cpp +++ b/vespamalloc/src/tests/thread/racemanythreads.cpp @@ -2,6 +2,7 @@ #include <vespa/vespalib/testkit/test_kit.h> #include <unistd.h> +#include <cassert> using namespace vespalib; @@ -9,9 +10,9 @@ void * hammer(void * arg) { usleep(4000000); long seconds = * static_cast<const long *>(arg); - long stopTime(time(NULL) + seconds); + long stopTime(time(nullptr) + seconds); pthread_t id = pthread_self(); - while (time(NULL) < stopTime) { + while (time(nullptr) < stopTime) { std::vector<pthread_t *> allocations; for (size_t i(0); i < 2000; i++) { pthread_t *t = new pthread_t[20]; @@ -21,11 +22,11 @@ void * hammer(void * arg) } } - for (size_t i(0); i < allocations.size(); i++) { + for (auto & allocation : allocations) { for (size_t j(0); j < 20; j++) { - assert(allocations[i][j] == id); + assert(allocation[j] == id); } - delete [] allocations[i]; + delete [] allocation; } } return arg; @@ -35,9 +36,9 @@ TEST_MAIN() { size_t threadCount(1024); long seconds(10); if (argc >= 2) { - threadCount = strtoul(argv[1], NULL, 0); + threadCount = strtoul(argv[1], nullptr, 0); if (argc >= 3) { - seconds = strtoul(argv[2], NULL, 0); + seconds = strtoul(argv[2], nullptr, 0); } } |