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

Lint Issues- Testing out addition of Check Destroy in some Acceptance Tests + other refactoring #998

Merged
merged 5 commits into from Apr 24, 2022

Conversation

Shocktrooper
Copy link
Collaborator

Resolves #433

@timofurrer
Copy link
Member

Can you remove the AT001 check from the provider lint check ignores:

TFPROVIDERLINTX_CHECKS = -XAT001=false -XR003=false -XS002=false

@timofurrer timofurrer added this to the v3.14.0 milestone Mar 31, 2022
@github-actions
Copy link

This pull request has merge conflicts. Please rebase your branch onto main.

@github-actions github-actions bot added the merge-conflict PR cannot be merged due to a merge conflict label Mar 31, 2022
# Conflicts:
#	internal/provider/resource_gitlab_project_test.go
@github-actions github-actions bot removed the merge-conflict PR cannot be merged due to a merge conflict label Mar 31, 2022
@github-actions
Copy link

Conflicts are resolved. Thank you! 😀

@Shocktrooper
Copy link
Collaborator Author

@timofurrer It appears that AT001 is different from XAT001 in the linter. Turning off XAT001 makes a massive amount of separate lint issues so leaving it for now.
https://github.com/gitlabhq/terraform-provider-gitlab/runs/5775213763?check_suite_focus=true
https://github.com/bflad/tfproviderlint/tree/main/xpasses/XAT001
https://github.com/bflad/tfproviderlint/tree/main/passes/AT001

@Shocktrooper
Copy link
Collaborator Author

@timofurrer I combed over the test case and am not sure why the Acceptance Test is failing. If you could take a look I think this is the last thing
https://github.com/gitlabhq/terraform-provider-gitlab/runs/5775393381?check_suite_focus=true#step:6:110

@Shocktrooper
Copy link
Collaborator Author

@armsnyder If you have a chance as well to take a look the issue is not apparent to me why it is failing.

@armsnyder
Copy link
Collaborator

Hi @Shocktrooper -- I couldn't tell what the issue was either. I assume it has to do with the new function you added to render out the Config field in each TestStep. Maybe try removing the TestSteps one by one, and running a targeted test to catch where the problem lies.

I also opened this PR to make the error more helpful in the future, not that it will help us now. 😄 hashicorp/terraform-plugin-sdk#925

@Shocktrooper Shocktrooper marked this pull request as ready for review April 3, 2022 21:50
@Shocktrooper
Copy link
Collaborator Author

Shocktrooper commented Apr 3, 2022

@armsnyder I figured out what steps at least. Apparently having a completely blank resource config could be the issue from what I can tell or it was the way the anonymous function was returned in the other Not Shared check function. After looking at the test again I didn't think it was needed either because we have a check destroy function so it should all be working now :) This is all ready for review again

@timofurrer
Copy link
Member

@Shocktrooper @armsnyder I've actually noticed that in other providers test where the have something like " " as a config to prevent this ...

@Shocktrooper
Copy link
Collaborator Author

Shocktrooper commented Apr 4, 2022

@timofurrer interesting do you have an example of this in action as I am unsure how that would be different than what I replaced in my last commit. Also is there a point to blank configs when you have a check destroy function.

Copy link
Member

@timofurrer timofurrer left a comment

Choose a reason for hiding this comment

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

LGTM 🎉

@timofurrer timofurrer merged commit b3732f9 into gitlabhq:main Apr 24, 2022
@Shocktrooper Shocktrooper deleted the lint-issue-AT001 branch April 25, 2022 00:57
@github-actions
Copy link

github-actions bot commented May 2, 2022

This functionality has been released in v3.14.0 of the Terraform GitLab Provider. Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading.

For further feature requests or bug reports with this functionality, please create a new GitHub issue. Thank you!

@github-actions github-actions bot locked and limited conversation to collaborators Nov 9, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
provider resource Adds or modifies a resource size/L tests
3 participants