TPT-4270: Integration tests for Linode integration with alerts#922
Conversation
psnoch-akamai
added
the
testing
psnoch-akamai
requested review from
ezilber-akamai and
lgarber-akamai
and removed request for
a team
There was a problem hiding this comment.
Pull request overview
Adds/updates integration tests to validate Linode instance alert fields (legacy + ACLP-related behaviors) and introduces fixtures to record the new workflows.
Changes:
- Add new integration workflows:
TestLinodeAlertsWorkflowandTestTryToUpdateLinodeAlertsLegacyAndAclpAtTheSameTime. - Update existing instance integration smoke/rebuild tests to assert alert fields are present.
- Add a
getRegionshelper and new recorded fixtures for the new integration tests.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| test/unit/instance_test.go | Adds a placeholder unit test stub for the “legacy + ACLP alerts” update scenario. |
| test/integration/integration_suite_test.go | Adds getRegions helper used by the new alert workflow tests. |
| test/integration/instances_test.go | Refactors some assertions to require, adds alert-field assertions, and adds 2 new alert-focused integration tests. |
| test/integration/fixtures/TestInstance_TryToUpdateLinodeAlertsLegacyAndAclpAtTheSameTime.yaml | New fixture covering the “cannot set both legacy and ACLP alerts” error case. |
| test/integration/fixtures/TestInstance_AlertsWorkflow.yaml | New fixture covering create → update alerts → clone workflow. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
psnoch-akamai
force-pushed
the
TPT-4270-linodego-qa-support-linode-integration-with-alerts
branch
from
a3b904e to
cddc607
Compare
…acyAndAclpAtTheSameTime and update existing tests to verify alerts fields
psnoch-akamai
force-pushed
the
TPT-4270-linodego-qa-support-linode-integration-with-alerts
branch
from
cddc607 to
97e11ef
Compare
psnoch-akamai
changed the title
psnoch-akamai
changed the title
| return result | ||
| } | ||
|
|
||
| func getRegions(t *testing.T, client *linodego.Client) []linodego.Region { |
There was a problem hiding this comment.
I think we should still use getRegionsWithCaps() instead of creating this one. At least filter by Linodes because the alerts will be used for Linodes.
| require.Equal(t, inst.PlacementGroup.PlacementGroupPolicy, pg.PlacementGroupPolicy) | ||
| } | ||
|
|
||
| func TestLinodeAlertsWorkflow(t *testing.T) { |
There was a problem hiding this comment.
This test case seems only tested the legacy alerts. Probably better to rename it and mention legacy alerts test in the name.
| require.NoError(t, err) | ||
| } | ||
|
|
||
| func TestTryToUpdateLinodeAlertsLegacyAndAclpAtTheSameTime(t *testing.T) { |
There was a problem hiding this comment.
Looks like we miss a test case for testing aclp alerts specifically. Can you add one for it?
psnoch-akamai
force-pushed
the
TPT-4270-linodego-qa-support-linode-integration-with-alerts
branch
from
067899d to
aec27f0
Compare
yec-akamai
left a comment
yec-akamai
left a comment
There was a problem hiding this comment.
New added test cases are passing. Nice work improve the old assertion style!
Can you update test instruction in the PR description? They are probably out-dated
| require.NoErrorf(t, err, "Error waiting for instance created: %s", err) | ||
|
|
||
| rebuildOpts := linodego.InstanceRebuildOptions{ | ||
| Image: "linode/alpine3.19", |
There was a problem hiding this comment.
Not quite related to this PR but I'm seeing failures when trying to re-run this PR's fixtures since linode/alpine3.19 has been EOL'd. Any chance we could bump all occurrences of linode/alpine3.19 to linode/alpine3.23 in this file?
There was a problem hiding this comment.
Good point, let me create new ticket for this upgrade https://track.akamai.com/jira/browse/TPT-4435
lgarber-akamai
left a comment
lgarber-akamai
left a comment
There was a problem hiding this comment.
Looks great, nice work!
psnoch-akamai
merged commit 3038288
into
linode:proj/aclp-linode-alerts
Create tests TestLinodeAlertsWorkflow, TestTryToUpdateLinodeAlertsLegacyAndAclpAtTheSameTime and update existing tests to verify alerts fields
✔️ How to Test
make test-int TEST_ARGS="-run TestInstances_List_smoke"
make test-int TEST_ARGS="-run TestInstance_Get_smoke"
make test-int TEST_ARGS="-run TestInstance_Rebuild"
make test-int TEST_ARGS="-run TestLinodeLegacyAlertsWorkflow"
make test-int TEST_ARGS="-run TestUpdateACLPAlerts"