Skip to content

Commit

Permalink
Upgrade to ginkgo v2, increase test coverage (cs3org#2526)
Browse files Browse the repository at this point in the history
* Upgrade to ginkgo v2

* Fix root info being counted twice

* Do not try to list child non-containers

* Improve generating response xml

* DRY up code

* Increase test coverage, port EncodePath benchmark to ginkgo

Also add an assertion that its performanc does not decrease too much.

* Add changelog

* Fix hound issue

* Add missing license header

* Fix linter issue
  • Loading branch information
aduffeck authored and butonic committed Feb 14, 2022
1 parent c2c5a45 commit f283b98
Show file tree
Hide file tree
Showing 64 changed files with 1,037 additions and 219 deletions.
3 changes: 3 additions & 0 deletions changelog/unreleased/upgrade-ginkgo.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
Enhancement: upgrade ginkgo to v2

https://github.com/cs3org/reva/pull/2526
4 changes: 2 additions & 2 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -48,8 +48,8 @@ require (
github.com/mitchellh/copystructure v1.2.0 // indirect
github.com/mitchellh/go-testing-interface v1.14.1 // indirect
github.com/mitchellh/mapstructure v1.4.3
github.com/onsi/ginkgo v1.16.5
github.com/onsi/gomega v1.18.0
github.com/onsi/ginkgo/v2 v2.0.0
github.com/onsi/gomega v1.18.1
github.com/pkg/errors v0.9.1
github.com/pkg/xattr v0.4.4
github.com/pquerna/cachecontrol v0.1.0 // indirect
Expand Down
10 changes: 3 additions & 7 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -158,7 +158,6 @@ github.com/fatih/color v1.9.0 h1:8xPHl4/q1VyqGIPif1F+1V3Y3lSmrq01EabUW3CoW5s=
github.com/fatih/color v1.9.0/go.mod h1:eQcE1qtQxscV5RaZvpXrrb8Drkc3/DdQ+uUYCNjL+zU=
github.com/form3tech-oss/jwt-go v3.2.2+incompatible/go.mod h1:pbq4aXjuKjdthFRnoDwaVPLA+WlJuPGy+QneDUgJi2k=
github.com/fsnotify/fsnotify v1.4.7/go.mod h1:jwhsz4b93w/PPRr/qN1Yymfu8t87LnFCMoQvtojpjFo=
github.com/fsnotify/fsnotify v1.4.9 h1:hsms1Qyu0jgnwNXIxa+/V/PDsU6CfLf6CNO8H7IWoS4=
github.com/fsnotify/fsnotify v1.4.9/go.mod h1:znqG4EE+3YCdAaPaxE2ZRY/06pZUdp0tY4IgpuI1SZQ=
github.com/gdexlab/go-render v1.0.1 h1:rxqB3vo5s4n1kF0ySmoNeSPRYkEsyHgln4jFIQY7v0U=
github.com/gdexlab/go-render v1.0.1/go.mod h1:wRi5nW2qfjiGj4mPukH4UV0IknS1cHD4VgFTmJX5JzM=
Expand Down Expand Up @@ -563,7 +562,6 @@ github.com/mwitkow/go-conntrack v0.0.0-20190716064945-2f068394615f/go.mod h1:qRW
github.com/nbio/st v0.0.0-20140626010706-e9e8d9816f32/go.mod h1:9wM+0iRr9ahx58uYLpLIr5fm8diHn0JbqRycJi6w0Ms=
github.com/niemeyer/pretty v0.0.0-20200227124842-a10e7caefd8e/go.mod h1:zD1mROLANZcx1PVRCS0qkT7pwLkGfwJo4zjcN/Tysno=
github.com/nxadm/tail v1.4.4/go.mod h1:kenIhsEOeOJmVchQTgglprH7qJGnHDVpk1VPCcaMI8A=
github.com/nxadm/tail v1.4.8 h1:nPr65rt6Y5JFSKQO7qToXr7pePgD6Gwiw05lkbyAQTE=
github.com/nxadm/tail v1.4.8/go.mod h1:+ncqLTQzXmGhMZNUePPaPqPvBxHAIsmXswZKocGu+AU=
github.com/oklog/run v1.0.0/go.mod h1:dlhp/R75TPv97u0XWUtDeV/lRKWPKSdTuV0TZvrmrQA=
github.com/oklog/run v1.1.0 h1:GEenZ1cK0+q0+wsJew9qUg/DyD8k3JzYsZAi5gYi2mA=
Expand All @@ -573,17 +571,16 @@ github.com/oklog/ulid v1.3.1/go.mod h1:CirwcVhetQ6Lv90oh/F+FBtV6XMibvdAFo93nm5qn
github.com/onsi/ginkgo v1.6.0/go.mod h1:lLunBs/Ym6LB5Z9jYTR76FiuTmxDTDusOGeTQH+WWjE=
github.com/onsi/ginkgo v1.7.0/go.mod h1:lLunBs/Ym6LB5Z9jYTR76FiuTmxDTDusOGeTQH+WWjE=
github.com/onsi/ginkgo v1.12.1/go.mod h1:zj2OWP4+oCPe1qIXoGWkgMRwljMUYCdkwsT2108oapk=
github.com/onsi/ginkgo v1.16.4 h1:29JGrr5oVBm5ulCWet69zQkzWipVXIol6ygQUe/EzNc=
github.com/onsi/ginkgo v1.16.4/go.mod h1:dX+/inL/fNMqNlz0e9LfyB9TswhZpCVdJM/Z6Vvnwo0=
github.com/onsi/ginkgo v1.16.5 h1:8xi0RTUf59SOSfEtZMvwTvXYMzG4gV23XVHOZiXNtnE=
github.com/onsi/ginkgo v1.16.5/go.mod h1:+E8gABHa3K6zRBolWtd+ROzc/U5bkGt0FwiG042wbpU=
github.com/onsi/ginkgo/v2 v2.0.0 h1:CcuG/HvWNkkaqCUpJifQY8z7qEMBJya6aLPx6ftGyjQ=
github.com/onsi/ginkgo/v2 v2.0.0/go.mod h1:vw5CSIxN1JObi/U8gcbwft7ZxR2dgaR70JSE3/PpL4c=
github.com/onsi/gomega v1.4.3/go.mod h1:ex+gbHU/CVuBBDIJjb2X0qEXbFg53c61hWP/1CpauHY=
github.com/onsi/gomega v1.7.1/go.mod h1:XdKZgCCFLUoM/7CFJVPcG8C1xQ1AJ0vpAezJrB7JYyY=
github.com/onsi/gomega v1.10.1/go.mod h1:iN09h71vgCQne3DLsj+A5owkum+a2tYe+TOCB1ybHNo=
github.com/onsi/gomega v1.17.0/go.mod h1:HnhC7FXeEQY45zxNK3PPoIUhzk/80Xly9PcubAlGdZY=
github.com/onsi/gomega v1.18.0 h1:ngbYoRctxjl8SiF7XgP0NxBFbfHcg3wfHMMaFHWwMTM=
github.com/onsi/gomega v1.18.0/go.mod h1:0q+aL8jAiMXy9hbwj2mr5GziHiwhAIQpFmmtT5hitRs=
github.com/onsi/gomega v1.18.1 h1:M1GfJqGRrBrrGGsbxzV5dqM2U2ApXefZCQpkukxYRLE=
github.com/onsi/gomega v1.18.1/go.mod h1:0q+aL8jAiMXy9hbwj2mr5GziHiwhAIQpFmmtT5hitRs=
github.com/opentracing/opentracing-go v1.1.0/go.mod h1:UkNAQd3GIcIGf0SeVgPpRdFStlNbqXla1AfSYxPUl2o=
github.com/opentracing/opentracing-go v1.2.0/go.mod h1:GxEUsuufX4nBwe+T+Wl9TAgYrxe9dPLANfrWvHYVTgc=
github.com/pascaldekloe/goe v0.0.0-20180627143212-57f6aae5913c/go.mod h1:lzWF7FIEvWOWxwDKqyGYQf6ZUaNfKdP144TG7ZOy1lc=
Expand Down Expand Up @@ -1192,7 +1189,6 @@ gopkg.in/ini.v1 v1.57.0 h1:9unxIsFcTt4I55uWluz+UmL95q4kdJ0buvQ1ZIqVQww=
gopkg.in/ini.v1 v1.57.0/go.mod h1:pNLf8WUiyNEtQjuu5G5vTm06TEv9tsIgeAvK8hOrP4k=
gopkg.in/square/go-jose.v2 v2.6.0 h1:NGk74WTnPKBNUhNzQX7PYcTLUjoq7mzKk2OKbvwk2iI=
gopkg.in/square/go-jose.v2 v2.6.0/go.mod h1:M9dMgbHiYLoDGQrXy7OpJDJWiKiU//h+vD76mk0e1AI=
gopkg.in/tomb.v1 v1.0.0-20141024135613-dd632973f1e7 h1:uRGJdciOHaEIrze2W8Q3AKkepLTh2hOroT7a+7czfdQ=
gopkg.in/tomb.v1 v1.0.0-20141024135613-dd632973f1e7/go.mod h1:dt/ZhP58zS4L8KSrWDmTeBkI65Dw0HsyUHuEVlX15mw=
gopkg.in/yaml.v2 v2.2.1/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI=
gopkg.in/yaml.v2 v2.2.2/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI=
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ package sharesstorageprovider_test
import (
"testing"

. "github.com/onsi/ginkgo"
. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"
)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ import (
"github.com/cs3org/reva/pkg/utils"
"google.golang.org/grpc"

. "github.com/onsi/ginkgo"
. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"
"github.com/stretchr/testify/mock"
)
Expand Down
59 changes: 59 additions & 0 deletions internal/http/services/owncloud/ocdav/net/context_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
// Copyright 2018-2022 CERN
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.
//
// In applying this license, CERN does not waive the privileges and immunities
// granted to it by virtue of its status as an Intergovernmental Organization
// or submit itself to any jurisdiction.

package net_test

import (
"context"

userpb "github.com/cs3org/go-cs3apis/cs3/identity/user/v1beta1"
"github.com/cs3org/reva/internal/http/services/owncloud/ocdav/net"
ctxpkg "github.com/cs3org/reva/pkg/ctx"

. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"
)

var _ = Describe("Net", func() {
var (
alice = &userpb.User{
Id: &userpb.UserId{
OpaqueId: "alice",
},
Username: "alice",
}
bob = &userpb.User{
Id: &userpb.UserId{
OpaqueId: "bob",
},
Username: "bob",
}
aliceCtx = ctxpkg.ContextSetUser(context.Background(), alice)
bobCtx = ctxpkg.ContextSetUser(context.Background(), bob)
)

Describe("IsCurrentUserOwner", func() {
It("returns true", func() {
Expect(net.IsCurrentUserOwner(aliceCtx, alice.Id)).To(BeTrue())
})

It("returns false", func() {
Expect(net.IsCurrentUserOwner(bobCtx, alice.Id)).To(BeFalse())
})
})
})
31 changes: 31 additions & 0 deletions internal/http/services/owncloud/ocdav/net/net_suite_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
// Copyright 2018-2021 CERN
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.
//
// In applying this license, CERN does not waive the privileges and immunities
// granted to it by virtue of its status as an Intergovernmental Organization
// or submit itself to any jurisdiction.

package net_test

import (
"testing"

. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"
)

func TestNet(t *testing.T) {
RegisterFailHandler(Fail)
RunSpecs(t, "Net Suite")
}
112 changes: 75 additions & 37 deletions internal/http/services/owncloud/ocdav/net/net_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,40 +16,78 @@
// granted to it by virtue of its status as an Intergovernmental Organization
// or submit itself to any jurisdiction.

package net

import "testing"

func TestParseDepth(t *testing.T) {
tests := map[string]Depth{
"": DepthOne,
"0": DepthZero,
"1": DepthOne,
"infinity": DepthInfinity,
}

for input, expected := range tests {
parsed, err := ParseDepth(input)
if err != nil {
t.Errorf("failed to parse depth %s", input)
}
if parsed != expected {
t.Errorf("parseDepth returned %s expected %s", parsed.String(), expected.String())
}
}

_, err := ParseDepth("invalid")
if err == nil {
t.Error("parse depth didn't return an error for invalid depth: invalid")
}
}

var result Depth

func BenchmarkParseDepth(b *testing.B) {
inputs := []string{"", "0", "1", "infinity", "INFINITY"}
size := len(inputs)
for i := 0; i < b.N; i++ {
result, _ = ParseDepth(inputs[i%size])
}
}
package net_test

import (
"time"

"github.com/cs3org/reva/internal/http/services/owncloud/ocdav/net"

. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"
. "github.com/onsi/gomega/gmeasure"
)

var _ = Describe("Net", func() {
DescribeTable("TestParseDepth",
func(v string, expectSuccess bool, expectedValue net.Depth) {
parsed, err := net.ParseDepth(v)
Expect(err == nil).To(Equal(expectSuccess))
Expect(parsed).To(Equal(expectedValue))
},
Entry("default", "", true, net.DepthOne),
Entry("0", "0", true, net.DepthZero),
Entry("1", "1", true, net.DepthOne),
Entry("infinity", "infinity", true, net.DepthInfinity),
Entry("invalid", "invalid", false, net.Depth("")))

Describe("ParseDepth", func() {
It("is reasonably fast", func() {
experiment := NewExperiment("Parsing depth headers")
AddReportEntry(experiment.Name, experiment)

inputs := []string{"", "0", "1", "infinity", "INFINITY"}
size := len(inputs)
experiment.Sample(func(i int) {
experiment.MeasureDuration("parsing", func() {
_, _ = net.ParseDepth(inputs[i%size])
})
}, SamplingConfig{Duration: time.Second})

encodingStats := experiment.GetStats("parsing")
medianDuration := encodingStats.DurationFor(StatMedian)

Expect(medianDuration).To(BeNumerically("<", 3*time.Millisecond))
})
})

Describe("EncodePath", func() {
It("encodes paths", func() {
Expect(net.EncodePath("foo")).To(Equal("foo"))
Expect(net.EncodePath("/some/path/Folder %^*(#1)")).To(Equal("/some/path/Folder%20%25%5e%2a(%231)"))
})

/*
The encodePath method as it is implemented currently is terribly inefficient.
As soon as there are a few special characters which need to be escaped the allocation count rises and the time spent too.
Adding more special characters increases the allocations and the time spent can rise up to a few milliseconds.
Granted this is not a lot on it's own but when a user has tens or hundreds of paths which need to be escaped and contain a few special characters
then this method alone will cost a huge amount of time.
*/
It("is reasonably fast", func() {
experiment := NewExperiment("Encoding paths")
AddReportEntry(experiment.Name, experiment)

experiment.Sample(func(idx int) {
experiment.MeasureDuration("encoding", func() {
_ = net.EncodePath("/some/path/Folder %^*(#1)")
})
}, SamplingConfig{Duration: time.Second})

encodingStats := experiment.GetStats("encoding")
medianDuration := encodingStats.DurationFor(StatMedian)

Expect(medianDuration).To(BeNumerically("<", 10*time.Millisecond))
})
})
})
2 changes: 1 addition & 1 deletion internal/http/services/owncloud/ocdav/ocdav_suite_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ package ocdav_test
import (
"testing"

. "github.com/onsi/ginkgo"
. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"
)

Expand Down
13 changes: 0 additions & 13 deletions internal/http/services/owncloud/ocdav/ocdav_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,19 +29,6 @@ import (
"github.com/cs3org/reva/pkg/utils/resourceid"
)

/*
The encodePath method as it is implemented currently is terribly inefficient.
As soon as there are a few special characters which need to be escaped the allocation count rises and the time spent too.
Adding more special characters increases the allocations and the time spent can rise up to a few milliseconds.
Granted this is not a lot on it's own but when a user has tens or hundreds of paths which need to be escaped and contain a few special characters
then this method alone will cost a huge amount of time.
*/
func BenchmarkEncodePath(b *testing.B) {
for i := 0; i < b.N; i++ {
_ = net.EncodePath("/some/path/Folder %^*(#1)")
}
}

func TestWrapResourceID(t *testing.T) {
expected := "c3RvcmFnZWlkOm9wYXF1ZWlk"
wrapped := resourceid.OwnCloudResourceIDWrap(&providerv1beta1.ResourceId{StorageId: "storageid", OpaqueId: "opaqueid"})
Expand Down
Loading

0 comments on commit f283b98

Please sign in to comment.