Tests update: Use require package in fleets package #1685

akremsa · 2020-07-09T11:20:40Z

What type of PR is this?

/kind bug
/kind cleanup

What this PR does / Why we need it:
Use require package to abort test in order to avoid nil pointer exceptions.

agones-bot · 2020-07-09T11:28:11Z

Build Failed 😱

Build Id: d6141283-019e-4e41-9b2a-0f613fc23143

To get permission to view the Cloud Build view, join the agones-discuss Google Group.

agones-bot · 2020-07-09T11:46:55Z

Build Failed 😱

Build Id: 3a818873-71a0-4428-a957-1d2140a17951

To get permission to view the Cloud Build view, join the agones-discuss Google Group.

agones-bot · 2020-07-09T13:35:10Z

Build Failed 😱

Build Id: c7b8b6fe-a487-4139-98ac-874309647f9d

To get permission to view the Cloud Build view, join the agones-discuss Google Group.

agones-bot · 2020-07-09T14:03:36Z

Build Succeeded 👏

Build Id: afea8c59-1e02-4ca1-ace6-ddc0e30ee8a9

The following development artifacts have been built, and will exist for the next 30 days:

image: gcr.io/agones-images/agones-controller:1.8.0-f692db8
image: gcr.io/agones-images/agones-sdk:1.8.0-f692db8
image: gcr.io/agones-images/agones-ping:1.8.0-f692db8
Linux C++ SDK (build): agonessdk-1.8.0-f692db8-linux-arch_64.tar.gz
SDK Server: agonessdk-server-1.8.0-f692db8.zip

A preview of the website (the last 30 builds are retained):

https://f692db8-dot-preview-dot-agones-images.appspot.com/

To install this version:

git fetch https://github.com/googleforgames/agones.git pull/1685/head:pr_1685 && git checkout pr_1685
helm install ./install/helm/agones --namespace agones-system --name agones --set agones.image.tag=1.8.0-f692db8

markmandel · 2020-07-09T18:58:43Z

I question if we should be replacing all asserts with require? Just thinking of other work that might be more valuable long term.

Or should we just replace the instances where we are doing variations of FailNow()?

akremsa · 2020-07-10T15:50:17Z

One benefit which I see from using assert is that it will show ALL failed cases if they exist at once. For example:

a := "a"
b := "b"
assert.Equal(t, "zzz", a)
assert.Equal(t, "zzz", b)

This code will output 2 errors for each assert at once which can be useful to see all possible failed test cases.
I prefer to use require only for those cases where we want to abort test execution.
On the other hand - sometimes it doesn't make sense to proceed with a test if one scenario is marked as Failed, but I think it's a developer's responsibility to write efficient tests.

markmandel · 2020-07-10T15:51:48Z

I prefer to use require only for those cases where we want to abort test execution.

100% agree 👍

fix

agones-bot · 2020-07-10T16:05:51Z

Build Failed 😱

Build Id: f13aebe0-b097-4048-8447-de09ca3373f1

To get permission to view the Cloud Build view, join the agones-discuss Google Group.

agones-bot · 2020-07-13T11:03:29Z

Build Failed 😱

Build Id: 74de68e2-12de-4ab5-b8e1-ed0c4b6f47b0

To get permission to view the Cloud Build view, join the agones-discuss Google Group.

markmandel · 2020-07-14T20:29:12Z

Does it make sense to close this PR? Since it looks like it won't be merged in it's current state? (sorry @akremsa !)

akremsa · 2020-07-14T21:58:26Z

@markmandel do you mean failed builds? Otherwise, this PR is perfectly fine, no?

markmandel · 2020-07-14T22:32:35Z

Let me come at this question another way - is all the places you've updated in this PR places where we want to abort execution of the test at the time of failure?

Maybe I misunderstood where we ended up.

akremsa · 2020-07-14T22:34:59Z

"is all the places you've updated in this PR places where we want to abort execution of the test at the time of failure"
Yes, exactly. I abort tests in cases where there is no point to continue.
And I suggest doing the same in the future.

markmandel · 2020-07-14T23:16:54Z

Ah! Got it. I will review this with that in mind. In which case, ignore my previous comment!

agones-bot · 2020-07-15T10:54:26Z

Build Succeeded 👏

Build Id: 670f8584-a6a7-4ead-8dab-429b01260000

The following development artifacts have been built, and will exist for the next 30 days:

image: gcr.io/agones-images/agones-controller:1.8.0-327f75b
image: gcr.io/agones-images/agones-sdk:1.8.0-327f75b
image: gcr.io/agones-images/agones-ping:1.8.0-327f75b
Linux C++ SDK (build): agonessdk-1.8.0-327f75b-linux-arch_64.tar.gz
SDK Server: agonessdk-server-1.8.0-327f75b.zip

A preview of the website (the last 30 builds are retained):

https://327f75b-dot-preview-dot-agones-images.appspot.com/

To install this version:

git fetch https://github.com/googleforgames/agones.git pull/1685/head:pr_1685 && git checkout pr_1685
helm install ./install/helm/agones --namespace agones-system --name agones --set agones.image.tag=1.8.0-327f75b

markmandel

LGTM!

google-oss-robot · 2020-07-16T21:39:37Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: akremsa, markmandel

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [markmandel]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

google-oss-robot · 2020-07-16T21:39:43Z

New changes are detected. LGTM label has been removed.

agones-bot · 2020-07-16T21:53:21Z

Build Failed 😱

Build Id: e2d881a9-f311-485d-aaaf-dd61fbe735dd

To get permission to view the Cloud Build view, join the agones-discuss Google Group.

markmandel · 2020-07-16T22:00:17Z

make[1]: *** [ensure-build-sdk-image] Error 2
includes/sdk.mk:171: recipe for target 'run-sdk-conformance-test-rest' failed

agones-bot · 2020-07-16T23:03:54Z

Build Succeeded 👏

Build Id: b8bd80d1-a50f-494c-a3ca-e2af7424cf94

The following development artifacts have been built, and will exist for the next 30 days:

image: gcr.io/agones-images/agones-controller:1.8.0-f704fa7
image: gcr.io/agones-images/agones-sdk:1.8.0-f704fa7
image: gcr.io/agones-images/agones-ping:1.8.0-f704fa7
Linux C++ SDK (build): agonessdk-1.8.0-f704fa7-linux-arch_64.tar.gz
SDK Server: agonessdk-server-1.8.0-f704fa7.zip

A preview of the website (the last 30 builds are retained):

https://f704fa7-dot-preview-dot-agones-images.appspot.com/

To install this version:

git fetch https://github.com/googleforgames/agones.git pull/1685/head:pr_1685 && git checkout pr_1685
helm install ./install/helm/agones --namespace agones-system --name agones --set agones.image.tag=1.8.0-f704fa7

) * use require in fleets controller_test Co-authored-by: Alexander Apalikov <alexander.apalikov@globant.com> Co-authored-by: Mark Mandel <markmandel@google.com>

googlebot added the cla: yes label Jul 9, 2020

google-oss-robot added the size/M label Jul 9, 2020

google-oss-robot requested review from dzlier-gcp and roberthbailey July 9, 2020 11:20

akremsa force-pushed the fix_fleets_controller_tests branch from 63acd22 to 108149f Compare July 9, 2020 11:30

akremsa changed the title ~~Tests update: Use require package in fleets/controller_test~~ Tests update: Use require package in fleets package Jul 9, 2020

akremsa force-pushed the fix_fleets_controller_tests branch from 935d333 to f692db8 Compare July 9, 2020 13:39

use require in fleets controller_test

a068754

fix

akremsa force-pushed the fix_fleets_controller_tests branch from f692db8 to 7befbf0 Compare July 10, 2020 15:55

use require in fleets_test

a6c1db2

akremsa force-pushed the fix_fleets_controller_tests branch from 7befbf0 to a6c1db2 Compare July 13, 2020 10:52

aLekSer mentioned this pull request Jul 15, 2020

CI: make test-gen-api-docs is failing quite often #1690

Closed

Merge branch 'master' into fix_fleets_controller_tests

327f75b

markmandel approved these changes Jul 16, 2020

View reviewed changes

google-oss-robot assigned markmandel Jul 16, 2020

google-oss-robot added the lgtm label Jul 16, 2020

google-oss-robot added the approved label Jul 16, 2020

Merge branch 'master' into fix_fleets_controller_tests

c2ee854

google-oss-robot removed the lgtm label Jul 16, 2020

Merge branch 'master' into fix_fleets_controller_tests

f704fa7

markmandel merged commit f9f4a26 into googleforgames:master Jul 17, 2020

markmandel added area/tests Unit tests, e2e tests, anything to make sure things don't break kind/cleanup Refactoring code, fixing up documentation, etc labels Jul 17, 2020

markmandel added this to the 1.8.0 milestone Jul 17, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Tests update: Use require package in fleets package #1685

Tests update: Use require package in fleets package #1685

akremsa commented Jul 9, 2020 •

edited

Loading

agones-bot commented Jul 9, 2020

agones-bot commented Jul 9, 2020

agones-bot commented Jul 9, 2020

agones-bot commented Jul 9, 2020

markmandel commented Jul 9, 2020

akremsa commented Jul 10, 2020 •

edited

Loading

markmandel commented Jul 10, 2020

agones-bot commented Jul 10, 2020

agones-bot commented Jul 13, 2020

markmandel commented Jul 14, 2020

akremsa commented Jul 14, 2020

markmandel commented Jul 14, 2020

akremsa commented Jul 14, 2020 •

edited

Loading

markmandel commented Jul 14, 2020

agones-bot commented Jul 15, 2020

markmandel left a comment

google-oss-robot commented Jul 16, 2020

google-oss-robot commented Jul 16, 2020

agones-bot commented Jul 16, 2020

markmandel commented Jul 16, 2020

agones-bot commented Jul 16, 2020

Tests update: Use require package in fleets package #1685

Tests update: Use require package in fleets package #1685

Conversation

akremsa commented Jul 9, 2020 • edited Loading

agones-bot commented Jul 9, 2020

agones-bot commented Jul 9, 2020

agones-bot commented Jul 9, 2020

agones-bot commented Jul 9, 2020

markmandel commented Jul 9, 2020

akremsa commented Jul 10, 2020 • edited Loading

markmandel commented Jul 10, 2020

agones-bot commented Jul 10, 2020

agones-bot commented Jul 13, 2020

markmandel commented Jul 14, 2020

akremsa commented Jul 14, 2020

markmandel commented Jul 14, 2020

akremsa commented Jul 14, 2020 • edited Loading

markmandel commented Jul 14, 2020

agones-bot commented Jul 15, 2020

markmandel left a comment

Choose a reason for hiding this comment

google-oss-robot commented Jul 16, 2020

google-oss-robot commented Jul 16, 2020

agones-bot commented Jul 16, 2020

markmandel commented Jul 16, 2020

agones-bot commented Jul 16, 2020

akremsa commented Jul 9, 2020 •

edited

Loading

akremsa commented Jul 10, 2020 •

edited

Loading

akremsa commented Jul 14, 2020 •

edited

Loading