Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

chore: fixtures: after delete() wait to verify deleted#1784

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged
nejch merged 1 commit intomainfromjlvillal/sidekiq
Jul 22, 2022

Conversation

@JohnVillalovos
Copy link
Member

In our fixtures that create:

  • groups
  • project merge requests
  • projects
  • users

They delete the created objects after use. Now wait to ensure the
objects are deleted before continuing as having unexpected objects
existing can impact some of our tests.

@JohnVillalovosJohnVillalovosforce-pushed thejlvillal/sidekiq branch 2 times, most recently fromd684c74 tob0b137aCompareJanuary 4, 2022 07:06
@JohnVillalovosJohnVillalovos marked this pull request as draftFebruary 12, 2022 16:53
@JohnVillalovosJohnVillalovos marked this pull request as ready for reviewJune 30, 2022 01:43
@JohnVillalovos
Copy link
MemberAuthor

Example of error in tests that this is trying to fix:
https://github.com/python-gitlab/python-gitlab/runs/7123034880?check_suite_focus=true

        group1 = gl.groups.create({"name": "group1", "path": "group1"})        group2 = gl.groups.create({"name": "group2", "path": "group2"})        p_id = gl.groups.list(search="group2")[0].id        group3 = gl.groups.create({"name": "group3", "path": "group3", "parent_id": p_id})        group4 = gl.groups.create({"name": "group4", "path": "group4"})>       assert len(gl.groups.list()) == 4E       assert 5 == 4E        +  where 5 = len([<Group id:35 name:group1>, <Group id:36 name:group2>, <Group id:37 name:group3>, <Group id:38 name:group4>, <Group id:6 name:test-group-45bec8b04d964c3fafd743afcb68f528>])E        +    where [<Group id:35 name:group1>, <Group id:36 name:group2>, <Group id:37 name:group3>, <Group id:38 name:group4>, <Group id:6 name:test-group-45bec8b04d964c3fafd743afcb68f528>] = <bound method ListMixin.list of <gitlab.v4.objects.groups.GroupManager object at 0x7f1e1b486ce0>>()E        +      where <bound method ListMixin.list of <gitlab.v4.objects.groups.GroupManager object at 0x7f1e1b486ce0>> = <gitlab.v4.objects.groups.GroupManager object at 0x7f1e1b486ce0>.listE        +        where <gitlab.v4.objects.groups.GroupManager object at 0x7f1e1b486ce0> = <gitlab.client.Gitlab object at 0x7f1e1b485d80>.groupstests/functional/api/test_groups.py:31: AssertionError

@JohnVillalovos
Copy link
MemberAuthor

@nejch FYI: I'd like to get this in before trying to get#1778 in. Also#1785 should get in too.

@nejch
Copy link
Member

@nejch FYI: I'd like to get this in before trying to get#1778 in. Also#1785 should get in too.

Thanks@JohnVillalovos I'll try to get back into the functional testing topic a bit.. I think we should make this reliable for sure, although it also shows we have some poor assertions in those legacy tests there.

IMO we should never be assertinglen() on any lists, just asserting on membership of those objects in the lists. That way our tests also wouldn't be as affected by async gitlab operations. E.g. if we create/delete a resource we shouldn't care if there are 9 or 10 of them on the server, only if that one exists or not :) Anyway, just some ramblings before I forget this, probably belongs in a separate issue :D

JohnVillalovos reacted with thumbs up emoji

@JohnVillalovosJohnVillalovos marked this pull request as draftJuly 3, 2022 19:30
@JohnVillalovosJohnVillalovosforce-pushed thejlvillal/sidekiq branch 6 times, most recently from80122a7 to7a86c8dCompareJuly 4, 2022 02:15
@JohnVillalovosJohnVillalovos marked this pull request as ready for reviewJuly 4, 2022 02:18
@JohnVillalovosJohnVillalovosforce-pushed thejlvillal/sidekiq branch 2 times, most recently from1dcf0f4 tob8d3aa5CompareJuly 20, 2022 22:18
@nejch
Copy link
Member

Thanks again@JohnVillalovos, just a few more random thoughts as I like that we're potentially even reducing the codebase here :)

In our fixtures that create:  - groups  - project merge requests  - projects  - usersThey delete the created objects after use. Now wait to ensure theobjects are deleted before continuing as having unexpected objectsexisting can impact some of our tests.
@nejchnejch merged commit916b1db intomainJul 22, 2022
@nejchnejch deleted the jlvillal/sidekiq branchJuly 22, 2022 05:08
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

@nejchnejchAwaiting requested review from nejch

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@JohnVillalovos@nejch

[8]ページ先頭

©2009-2025 Movatter.jp