Skip to content
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix failing test #123

Merged
merged 10 commits into from
Apr 2, 2024
Merged

Fix failing test #123

merged 10 commits into from
Apr 2, 2024

Conversation

drselump14
Copy link
Contributor

what

Fix the failing test because outdated code

why

Test is failing on most of pull request

references

@drselump14 drselump14 mentioned this pull request Mar 7, 2024
@hans-d hans-d self-assigned this Mar 7, 2024
@hans-d
Copy link

hans-d commented Mar 7, 2024

/terratest

@drselump14
Copy link
Contributor Author

drselump14 commented Mar 7, 2024

updated the readme

@hans-d
Copy link

hans-d commented Mar 7, 2024

/terratest

@hans-d
Copy link

hans-d commented Mar 7, 2024

Likely failing tesrratest due to some of the ancient versions involved.
@drselump14 can you see if you can get some decent versions here? I wont be able to process that bit, but this issue in general has my attention and working on a cross-repo fix re how we call the tests and can run them more easily locally.

@drselump14
Copy link
Contributor Author

@hans-d I updated the terraform versions to follow the versions on https://github.com/cloudposse/terraform-aws-vpc/blob/main/versions.tf

I only checked if terraform apply works on the examples folder. Not really sure how to invoke the test script as it's always failed with different errors on local

@hans-d
Copy link

hans-d commented Mar 7, 2024

Not really sure how to invoke the test script as it's always failed with different errors on local
no worries, we try to make that simpler (and better documented) as well.

@hans-d
Copy link

hans-d commented Mar 7, 2024

/terratest

@hans-d
Copy link

hans-d commented Mar 7, 2024

I'll have alook later re the failing test. Bit of a beast that one

@hans-d
Copy link

hans-d commented Mar 10, 2024

@drselump14 Hi, can you update the this

so that it want TF >= 1?

That should resolve the test from failing. (and we need a new readme)

@hans-d hans-d added help wanted Extra attention is needed bugfix Change that restores intended behavior labels Mar 10, 2024
Copy link

mergify bot commented Mar 10, 2024

This pull request now has conflicts. Could you fix it @drselump14? 🙏

@mergify mergify bot added the conflict This PR has conflicts label Mar 10, 2024
Copy link

mergify bot commented Mar 10, 2024

Thanks @drselump14 for creating this pull request!

A maintainer will review your changes shortly. Please don't be discouraged if it takes a while.

While you wait, make sure to review our contributor guidelines.

Tip

Need help or want to ask for a PR review to be expedited?

Join us on Slack in the #pr-reviews channel.

@mergify mergify bot added the needs-test Needs testing label Mar 11, 2024
@mergify mergify bot removed the conflict This PR has conflicts label Mar 11, 2024
@drselump14
Copy link
Contributor Author

@drselump14 Hi, can you update the this


so that it want TF >= 1?

That should resolve the test from failing. (and we need a new readme)

Done ✅

@hans-d
Copy link

hans-d commented Mar 11, 2024

/terratest

@hans-d hans-d removed the help wanted Extra attention is needed label Mar 11, 2024
hans-d
hans-d previously approved these changes Mar 11, 2024
@hans-d hans-d enabled auto-merge (squash) March 11, 2024 10:51
@hans-d hans-d added the no-release Do not create a new release (wait for additional code changes) label Mar 11, 2024
@kevcube
Copy link

kevcube commented Mar 28, 2024

@drselump14 still failing, the error is a go error. I think possibly one output is not formatted as expected? I'll dig in later today.

@Roondel
Copy link
Contributor

Roondel commented Apr 1, 2024

@drselump14 still failing, the error is a go error. I think possibly one output is not formatted as expected? I'll dig in later today.

@kevcube where you able to dig into this at all? I am fighting the same error on PR #121

what I saw when I tried to trigger the test locally using go cli was an error for unmarshalling the json:
FAIL: TestExamplesComplete (366.65s)
test_examples_complete_test.go:42:
Error Trace: test_examples_complete_test.go:42
Error: Received unexpected error:
json: cannot unmarshal string into Go value of type map[string]interface {}
Test: TestExamplesComplete
test_examples_complete_test.go:43:
Error Trace: test_examples_complete_test.go:43
Error: Not equal:
expected: string("geodesic")
actual : ()
Test: TestExamplesComplete
test_examples_complete_test.go:44:
Error Trace: test_examples_complete_test.go:44
Error: Not equal:
expected: string("cloudposse/geodesic")
actual : ()
Test: TestExamplesComplete
panic: interface conversion: interface {} is nil, not float64 [recovered]
panic: interface conversion: interface {} is nil, not float64

@kevcube
Copy link

kevcube commented Apr 2, 2024

@Roondel I think terratest needs to be upgraded in this repo. I will try that now

@kevcube
Copy link

kevcube commented Apr 2, 2024

/terratest

@kevcube kevcube added minor New features that do not break anything and removed no-release Do not create a new release (wait for additional code changes) labels Apr 2, 2024
@kevcube
Copy link

kevcube commented Apr 2, 2024

removing no-release tag because we are updating a dependency

Copy link

@gberenice gberenice left a comment

Choose a reason for hiding this comment

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

:shipit: 🚀

@hans-d hans-d merged commit b2a0b9c into cloudposse:main Apr 2, 2024
27 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bugfix Change that restores intended behavior minor New features that do not break anything needs-test Needs testing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants