-
Notifications
You must be signed in to change notification settings - Fork 724
Fix Console History tests #221
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
Conversation
* refine instance fixtures to use fastest VM
| } | ||
|
|
||
| func (s *ConsoleHistoryResourceCrud) Delete() (e error) { | ||
| e = nil |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm probably missing some context. It looks like this would silently succeed without doing the delete?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A few lines up in deleteConsoleHistory the return fmt.Errorf("console history resource: console history %v cannot be deleted", d.Id()) line was causing the test to fail even after the test assertion succeeded. It also indicated the resource could not be deleted, this is the pattern for that. Looking at the api docs, there is a DeleteConsoleHistory api--probably added after this was developed a year ago, but I'm more inclined to remove this feature and tests than invest more in it--there is no use case for console history in terraform, it was originally added by mistake.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Makes sense - I agree that there isn't much of a use case here. If it's possible to log a warning here to say that delete isn't supported, that would be good.
| display_name = "network_name" | ||
| } | ||
| resource "baremetal_core_internet_gateway" "CompleteIG" { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why are these being removed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not used in the test at all--added setup time for fixtures.
…8 to dev
Squashed commit of the following:
commit f924eebeb4d95a3e88bd49f481f0e4b42b73f5aa
Author: Brian Gustafson <brian.gustafson@oracle.com>
Date: Thu May 10 15:42:20 2018 -0700
Reference example in changelog
commit 0fba0039bfa4d57204e622bce057a99d760c111c
Author: Brian Gustafson <brian.gustafson@oracle.com>
Date: Thu May 10 14:32:54 2018 -0700
Changelog for 2.1.8
TESTS:
TF_ACC=1 TF_ORACLE_ENV=test ./terraform-plain.sh -timeout 120m -run TestResourceCoreConsoleHistoryTestSuiteTF_ACC=1 TF_ORACLE_ENV=test ./terraform-plain.sh -timeout 120m -run TestDatasourceCoreConsoleHistoryTestSuite