From 43bd1d6620f24b55833bd9500a44d694234b3453 Mon Sep 17 00:00:00 2001 From: Xavier Coulon Date: Tue, 3 Oct 2017 10:28:46 +0200 Subject: [PATCH] Stabilize/disable tests related to if_modified_since_header (#1655) Using the `Last-Modified` response header when possible Also, making sure that the list of spaces is always ordered by `updated_at DESC` to get the most recents spaces first. Fixes #1655 Signed-off-by: Xavier Coulon --- controller/search_spaces_blackbox_test.go | 25 ++++++++++--------- controller/space_blackbox_test.go | 6 +++-- .../work_item_children_blackbox_test.go | 6 +++-- space/space.go | 2 ++ 4 files changed, 23 insertions(+), 16 deletions(-) diff --git a/controller/search_spaces_blackbox_test.go b/controller/search_spaces_blackbox_test.go index eaa7ed935a..c65a5fe581 100644 --- a/controller/search_spaces_blackbox_test.go +++ b/controller/search_spaces_blackbox_test.go @@ -6,6 +6,7 @@ import ( "strconv" "strings" "testing" + "time" "github.com/fabric8-services/fabric8-wit/app" "github.com/fabric8-services/fabric8-wit/app/test" @@ -42,7 +43,6 @@ type okScenario struct { type TestSearchSpacesREST struct { gormtestsupport.DBTestSuite - db *gormapplication.GormDB clean func() } @@ -73,18 +73,19 @@ func (rest *TestSearchSpacesREST) UnSecuredController() (*goa.Service, *SearchCo func (rest *TestSearchSpacesREST) TestSpacesSearchOK() { // given - idents, err := createTestData(rest.db) + prefix := time.Now().Format("2006_Jan_2_15_04_05_") // using a unique prefix to make sure the test data will not collide with existing, older spaces. + idents, err := createTestData(rest.db, prefix) require.Nil(rest.T(), err) tests := []okScenario{ - {"With uppercase fullname query", args{offset("0"), limit(10), "TEST_AB"}, expects{totalCount(1)}}, - {"With lowercase fullname query", args{offset("0"), limit(10), "TEST_AB"}, expects{totalCount(1)}}, - {"With uppercase description query", args{offset("0"), limit(10), "DESCRIPTION FOR TEST_AB"}, expects{totalCount(1)}}, - {"With lowercase description query", args{offset("0"), limit(10), "description for test_ab"}, expects{totalCount(1)}}, + {"With uppercase fullname query", args{offset("0"), limit(10), prefix + "TEST_AB"}, expects{totalCount(1)}}, + {"With lowercase fullname query", args{offset("0"), limit(10), prefix + "TEST_AB"}, expects{totalCount(1)}}, + {"With uppercase description query", args{offset("0"), limit(10), "DESCRIPTION FOR " + prefix + "TEST_AB"}, expects{totalCount(1)}}, + {"With lowercase description query", args{offset("0"), limit(10), "description for " + prefix + "test_ab"}, expects{totalCount(1)}}, {"with special chars", args{offset("0"), limit(10), "&:\n!#%?*"}, expects{totalCount(0)}}, {"with * to list all", args{offset("0"), limit(10), "*"}, expects{totalCountAtLeast(len(idents))}}, - {"with multi page", args{offset("0"), limit(10), "TEST"}, expects{hasLinks("Next")}}, - {"with last page", args{offset(strconv.Itoa(len(idents) - 1)), limit(10), "TEST"}, expects{hasNoLinks("Next"), hasLinks("Prev")}}, - {"with different values", args{offset("0"), limit(10), "TEST"}, expects{differentValues()}}, + {"with multi page", args{offset("0"), limit(10), prefix + "TEST"}, expects{hasLinks("Next")}}, + {"with last page", args{offset(strconv.Itoa(len(idents) - 1)), limit(10), prefix + "TEST"}, expects{hasNoLinks("Next"), hasLinks("Prev")}}, + {"with different values", args{offset("0"), limit(10), prefix + "TEST"}, expects{differentValues()}}, } svc, ctrl := rest.UnSecuredController() // when/then @@ -96,10 +97,10 @@ func (rest *TestSearchSpacesREST) TestSpacesSearchOK() { } } -func createTestData(db application.DB) ([]space.Space, error) { - names := []string{"TEST_A", "TEST_AB", "TEST_B", "TEST_C"} +func createTestData(db application.DB, prefix string) ([]space.Space, error) { + names := []string{prefix + "TEST_A", prefix + "TEST_AB", prefix + "TEST_B", prefix + "TEST_C"} for i := 0; i < 20; i++ { - names = append(names, "TEST_"+strconv.Itoa(i)) + names = append(names, prefix+"TEST_"+strconv.Itoa(i)) } spaces := []space.Space{} diff --git a/controller/space_blackbox_test.go b/controller/space_blackbox_test.go index 9d8f5cc86e..06f7c47df3 100644 --- a/controller/space_blackbox_test.go +++ b/controller/space_blackbox_test.go @@ -550,9 +550,11 @@ func (rest *TestSpaceREST) TestListSpacesOKUsingExpiredIfModifiedSinceHeader() { p := minimumRequiredCreateSpace() p.Data.Attributes.Name = &name svc, ctrl := rest.SecuredController(testsupport.TestIdentity) - test.CreateSpaceCreated(rest.T(), svc.Context, svc, ctrl, p) + _, createdSpace := test.CreateSpaceCreated(rest.T(), svc.Context, svc, ctrl, p) // when - ifModifiedSince := app.ToHTTPTime(time.Now().Add(-1 * time.Hour)) + rest.T().Logf("space created at=%s", createdSpace.Data.Attributes.CreatedAt.UTC().String()) + ifModifiedSince := app.ToHTTPTime(createdSpace.Data.Attributes.CreatedAt.Add(-1 * time.Hour)) + rest.T().Logf("requesting with `If-Modified-Since`=%s", ifModifiedSince) _, list := test.ListSpaceOK(rest.T(), svc.Context, svc, ctrl, nil, nil, &ifModifiedSince, nil) // then require.NotNil(rest.T(), list) diff --git a/controller/work_item_children_blackbox_test.go b/controller/work_item_children_blackbox_test.go index c171fe94a2..5fc3c4aa83 100644 --- a/controller/work_item_children_blackbox_test.go +++ b/controller/work_item_children_blackbox_test.go @@ -276,9 +276,11 @@ func (s *workItemChildSuite) TestChildren() { assertResponseHeaders(t, res) }) s.T().Run("not modified using if modified since header", func(t *testing.T) { + // given + res, _ := test.ListChildrenWorkitemOK(t, s.svc.Context, s.svc, s.workItemCtrl, *s.bug1.Data.ID, nil, nil, nil, nil) + ifModifiedSince := res.Header()[app.LastModified][0] // when - ifModifiedSince := app.ToHTTPTime(s.bug3.Data.Attributes[workitem.SystemUpdatedAt].(time.Time)) - res := test.ListChildrenWorkitemNotModified(t, s.svc.Context, s.svc, s.workItemCtrl, *s.bug1.Data.ID, nil, nil, &ifModifiedSince, nil) + res = test.ListChildrenWorkitemNotModified(t, s.svc.Context, s.svc, s.workItemCtrl, *s.bug1.Data.ID, nil, nil, &ifModifiedSince, nil) // then assertResponseHeaders(t, res) }) diff --git a/space/space.go b/space/space.go index 192168d708..8441e06c75 100644 --- a/space/space.go +++ b/space/space.go @@ -258,6 +258,8 @@ func (r *GormRepository) listSpaceFromDB(ctx context.Context, q *string, userID db = db.Where("spaces.owner_id=?", userID) } + // ensure that the result list is always ordered in the same manner + db = db.Order("spaces.updated_at DESC") rows, err := db.Rows() if err != nil { return nil, 0, errs.WithStack(err)