Skip to content

correcting runtime versions in integ test#8947

Open
Vandita2020 wants to merge 2 commits intoaws:developfrom
Vandita2020:integ
Open

correcting runtime versions in integ test#8947
Vandita2020 wants to merge 2 commits intoaws:developfrom
Vandita2020:integ

Conversation

@Vandita2020
Copy link
Copy Markdown
Contributor

Which issue(s) does this change fix?

NA

Why is this change necessary?

Fixes the integration test failure:
terraform-build
other-and-e2e
tier1-windows-build-2

How does it address the issue?

What side effects does this change have?

Mandatory Checklist

PRs will only be reviewed after checklist is complete

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@Vandita2020 Vandita2020 requested a review from a team as a code owner May 5, 2026 22:41
@vicheey
Copy link
Copy Markdown
Contributor

vicheey commented May 5, 2026

Is there an integration test running for these updated cases?

@roger-zhangg roger-zhangg reopened this May 5, 2026
@Vandita2020 Vandita2020 changed the base branch from develop to nightly-builds May 6, 2026 19:59
@Vandita2020 Vandita2020 changed the base branch from nightly-builds to develop May 6, 2026 21:12
Copy link
Copy Markdown
Collaborator

@aws-sam-cli-bot aws-sam-cli-bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review Results

Reviewed: a40fb27..427bec6
Files: 14
Comments: 1


Comments on lines outside the diff:

[tests/integration/validate/test_validate_command.py:214] [GENERAL] nodejs20.x was removed from supported_runtimes, but it was not added to the test_lint_deprecated_runtimes parametrize list (currently: nodejs, python3.9, nodejs18.x, ruby3.2, dotnet6). If the reason for the removal is that cfn-lint now reports nodejs20.x as deprecated (the most likely cause of the test_lint_supported_runtimes failure this PR is fixing), then dropping it from the supported list without adding it to the deprecated list means the project loses all linter coverage for that runtime. Consider adding it to the deprecated cases so the deprecation path is actually exercised:

@parameterized.expand(
   [
       ("nodejs",),
       ("python3.9",),
       ("nodejs18.x",),
       ("nodejs20.x",),
       ("ruby3.2",),
       ("dotnet6",),
   ]
)
def test_lint_deprecated_runtimes(self, runtime):

This also addresses the unresolved question from the previous review about integration-test coverage for the updated runtime cases. If nodejs20.x is not yet fully deprecated by cfn-lint (only warning, etc.), please note the intent in the PR so the gap is explicit.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants