[WPB-5687] port flaking LH tests to new integration #3876

MangoIV · 2024-02-06T14:50:51Z

https://wearezeta.atlassian.net/browse/WPB-5687

Checklist

Add a new entry in an appropriate subdirectory of changelog.d
Read and follow the PR guidelines

integration/test/API/GalleyInternal.hs

integration/test/MLS/Util.hs

integration/test/Testlib/MockIntegrationService.hs

MangoIV · 2024-02-13T14:08:35Z

/teams/{tid}/legalhod/settings not ported because they don't flake.

- delegate only the testcase generation to the user - use an OVERLAPPABLE default instance if the type is a Generic Enum - cover more cases - don't use newtype Wrappers wherever possible

MangoIV · 2024-02-14T15:17:45Z

I already have a flaking test: LegalHold.testLHDisableForUser, it is extremely flaky though so I'm going to have to fix it :3

MangoIV · 2024-02-15T23:30:06Z

still needs some cleanups (also for some reason postProteus reports 404 no endpoint) :(

MangoIV · 2024-02-16T09:55:26Z

client removed notification takes extremely long to arrive; on local it was fine to bump the timeout, in CI it takes over 20 seconds for it before it times out

MangoIV · 2024-02-20T14:59:19Z

I have no idea on how to solve this issue with timeouts, it's because the SQS queue is really slow. there's two options:

bump the timeouts to something really high
don't wait for these events

because of message queueing

.hlint.yaml

integration/test/Testlib/HTTP.hs

integration/test/Testlib/ModService.hs

MangoIV · 2024-02-21T12:29:49Z

there are still some legalhold tests that are not ported but I have confirmed that these have not flaked for some time (sinche the http-client revert)

integration/test/Notifications.hs

integration/test/API/Brig.hs

fisx · 2024-02-21T13:18:25Z

integration/test/API/Brig.hs

@@ -149,7 +149,6 @@ deleteUser user = do
  submit "DELETE" $
    req & addJSONObject ["password" .= defPassword]

-- | https://staging-nginz-https.zinfra.io/api-internal/swagger-ui/brig/#/brig/post_i_clients__uid_


removed by accident?

no, this is the wrong endpoint (internal)

I have replaced this with the proper one

integration/test/API/BrigInternal.hs

integration/test/API/Galley.hs

integration/test/API/GalleyInternal.hs

integration/test/Testlib/PTest.hs

integration/test/Testlib/MockIntegrationService.hs

sorry, i wanted to just send the comments, i approved by accident.

Co-authored-by: Matthias Fischmann <[email protected]>

integration/test/Test/LegalHold.hs

fisx · 2024-02-21T15:36:27Z

integration/test/Test/LegalHold.hs

+        postLegalHoldSettings tid alice (mkLegalHoldSettings lhPort)
+          >>= assertStatus 201
+
+        if not connectFirst


I also stumbled over this in the old test but I does make sense.

integration/test/Test/LegalHold.hs

pcapriotti

LGTM. I only skimmed over the new tests, but they seem ok to me. I like the improvements to the HasTests class. Personally, I would have skipped the Generics stuff, but I'm not going to fight you on that :).

* [fix] use -e flag to abort when `docker-compose` fails * [feat] make `HasTests` easier to use - delegate only the testcase generation to the user - use an OVERLAPPABLE default instance if the type is a Generic Enum - cover more cases - don't use newtype Wrappers wherever possible * [feat] port over flaking Legalhold tests and delete them from galley integration * [feat] minor testlib improvements and additions --------- Co-authored-by: Matthias Fischmann <[email protected]>

* WPB-5845 guests should not be able to join conversations under legalhold (#3853) * Clean up LH tests (#3830) * Use HasTests to save a few LOC. * Fix/extend client CRUD api. - moved internal add from API.Brig to API.BrigInternal - created API.BrigCommon for data structured needed in both - added public add * Tranlate tests: manually add/delete client. * Fiddle with test case type abstractions. * Remove obsolete test from integration/test/Test/Demo.hs * Unblock release. (#3871) * WIP: [WPB-5687] port flaking LH tests to new integration (#3876) * [fix] use -e flag to abort when `docker-compose` fails * [feat] make `HasTests` easier to use - delegate only the testcase generation to the user - use an OVERLAPPABLE default instance if the type is a Generic Enum - cover more cases - don't use newtype Wrappers wherever possible * [feat] port over flaking Legalhold tests and delete them from galley integration * [feat] minor testlib improvements and additions --------- Co-authored-by: Matthias Fischmann <[email protected]> * wip * fix integration tests --------- Co-authored-by: fisx <[email protected]> Co-authored-by: Mango The Fourth <[email protected]>

MangoIV changed the title ~~Mangoiv/lh tests to new integration~~ [WPB-5687] lh tests to new integration Feb 6, 2024

MangoIV changed the title ~~[WPB-5687] lh tests to new integration~~ [WPB-5687] port flaking LH tests to new integration Feb 6, 2024

zebot added the ok-to-test Approved for running tests in CI, overrides not-ok-to-test if both labels exist label Feb 6, 2024

MangoIV force-pushed the mangoiv/lh-tests-to-new-integration branch from 8a4756c to 010a6fc Compare February 6, 2024 16:37

MangoIV added 3 commits February 12, 2024 10:08

[fix] use -e flag to abort when docker-compose fails

05d92f2

[feat] port testRequestLegalHoldDevice

4e4e9d1

[wip] implement add Legalhold Device

44e1f4a

MangoIV force-pushed the mangoiv/lh-tests-to-new-integration branch from 010a6fc to 44e1f4a Compare February 12, 2024 09:10

[feat] test no. 2 and some fixes on test no. 1

6fc66a1

MangoIV commented Feb 12, 2024

View reviewed changes

MangoIV added 2 commits February 13, 2024 12:38

[feat] port over test number 3

c3c52c8

[feat] test number 4

6ffcac6

[fix] don't depend on the order of notifications

66a4242

MangoIV force-pushed the mangoiv/lh-tests-to-new-integration branch from 9ebaf36 to 66a4242 Compare February 13, 2024 14:58

MangoIV added 3 commits February 13, 2024 17:47

[feat] test number 5

5e0e657

[feat] test number 6

b24f806

[feat] make HasTests easier to use

b1b3af0

- delegate only the testcase generation to the user - use an OVERLAPPABLE default instance if the type is a Generic Enum - cover more cases - don't use newtype Wrappers wherever possible

MangoIV added 3 commits February 14, 2024 18:58

[wip] test no 7

b3b8d03

[feat] test number 7 and (hopefull) fix flakes

8892000

[chore] delete ported test

0082396

MangoIV added 2 commits February 16, 2024 10:56

[test] REVERT: set ridiculously high timeout

f25989a

[fix] fix message receiving not working

84feffa

MangoIV force-pushed the mangoiv/lh-tests-to-new-integration branch from 56b6955 to 84feffa Compare February 19, 2024 12:25

[fix] set high timeout for events for tests that don't work reliably

402d6ec

because of message queueing

MangoIV force-pushed the mangoiv/lh-tests-to-new-integration branch from 392d4a6 to 402d6ec Compare February 20, 2024 15:28

[fix] some minor fixups

a193a78

MangoIV marked this pull request as ready for review February 21, 2024 11:54

MangoIV requested a review from fisx February 21, 2024 12:13

MangoIV commented Feb 21, 2024

View reviewed changes

.hlint.yaml Show resolved Hide resolved

integration/test/Testlib/HTTP.hs Show resolved Hide resolved

integration/test/Testlib/ModService.hs Show resolved Hide resolved

MangoIV force-pushed the mangoiv/lh-tests-to-new-integration branch 2 times, most recently from 6938583 to f4c8b09 Compare February 21, 2024 13:39

MangoIV commented Feb 21, 2024

View reviewed changes

integration/test/Notifications.hs Show resolved Hide resolved

integration/test/Notifications.hs Show resolved Hide resolved

fisx previously approved these changes Feb 21, 2024

View reviewed changes

[feat] change location for local, fix bug in time measuring for notifs

df9ce7f

MangoIV force-pushed the mangoiv/lh-tests-to-new-integration branch from f4c8b09 to df9ce7f Compare February 21, 2024 14:06

Update integration/test/Testlib/PTest.hs

25cf9bf

Co-authored-by: Matthias Fischmann <[email protected]>

fisx approved these changes Feb 21, 2024

View reviewed changes

[fix] implement all the suggestions by @fisx

fe70861

MangoIV force-pushed the mangoiv/lh-tests-to-new-integration branch from 16a2d97 to fe70861 Compare February 22, 2024 14:05

pcapriotti approved these changes Feb 22, 2024

View reviewed changes

[chore] add changelog entry

ecc3562

MangoIV merged commit c6acc1f into develop Feb 23, 2024
8 checks passed

MangoIV deleted the mangoiv/lh-tests-to-new-integration branch February 23, 2024 20:23

zebot mentioned this pull request Apr 25, 2024

Release 2024-04-25 - (expected chart version 4.42.0) #4020

Merged

echoes-hq bot added the echoes: throughput/ci-maintenance label Jun 28, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[WPB-5687] port flaking LH tests to new integration #3876

[WPB-5687] port flaking LH tests to new integration #3876

MangoIV commented Feb 6, 2024 •

edited

Loading

MangoIV commented Feb 13, 2024

MangoIV commented Feb 14, 2024

MangoIV commented Feb 15, 2024

MangoIV commented Feb 16, 2024

MangoIV commented Feb 20, 2024

MangoIV commented Feb 21, 2024

fisx Feb 21, 2024

MangoIV Feb 22, 2024

MangoIV Feb 22, 2024

fisx Feb 21, 2024

MangoIV Feb 21, 2024

pcapriotti left a comment

[WPB-5687] port flaking LH tests to new integration #3876

[WPB-5687] port flaking LH tests to new integration #3876

Conversation

MangoIV commented Feb 6, 2024 • edited Loading

Checklist

MangoIV commented Feb 13, 2024

MangoIV commented Feb 14, 2024

MangoIV commented Feb 15, 2024

MangoIV commented Feb 16, 2024

MangoIV commented Feb 20, 2024

MangoIV commented Feb 21, 2024

fisx Feb 21, 2024

Choose a reason for hiding this comment

MangoIV Feb 22, 2024

Choose a reason for hiding this comment

MangoIV Feb 22, 2024

Choose a reason for hiding this comment

fisx Feb 21, 2024

Choose a reason for hiding this comment

MangoIV Feb 21, 2024

Choose a reason for hiding this comment

pcapriotti left a comment

Choose a reason for hiding this comment

MangoIV commented Feb 6, 2024 •

edited

Loading