Skip to content

Conversation

@stevehipwell
Copy link
Collaborator

@stevehipwell stevehipwell commented Dec 11, 2024

Related to #2425


Before the change?

  • There isn't a way to run acceptance tests from a fork PR
  • Linting is showing warning
  • Workflows aren't following best practices for security hardening

After the change?

  • Acceptance tests can be run from a fork PR
  • Linting isn't showing warning
  • Workflows have been hardened

Pull request checklist

  • Tests for the changes have been added (for bug fixes / features)
  • Docs have been reviewed and added / updated if needed (for bug fixes / features)

Does this introduce a breaking change?

Please see our docs on breaking changes to help!

  • Yes
  • No

Signed-off-by: Steve Hipwell <steve.hipwell@gmail.com>
@github-actions
Copy link

👋 Hey Friends, this pull request has been automatically marked as stale because it has no recent activity. It will be closed if no further activity occurs. Please add the Status: Pinned label if you feel that this issue needs to remain open/active. Thank you for your contributions and help in keeping things tidy!

@github-actions github-actions bot added the Status: Stale Used by stalebot to clean house label Oct 15, 2025
@stevehipwell
Copy link
Collaborator Author

/not-stale

@github-actions github-actions bot removed the Status: Stale Used by stalebot to clean house label Oct 16, 2025
@nickfloyd nickfloyd moved this from Backlog to In Progress in Terraform Provider Oct 21, 2025
Copy link
Contributor

@deiga deiga left a comment

Choose a reason for hiding this comment

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

This is looking quite good! What would it take to get this finished?

GITHUB_TEST_EXTERNAL_USER_TOKEN: ""
GITHUB_TEST_EXTERNAL_USER2: ""
GITHUB_TEST_ADVANCED_SECURITY: "true"
run: go test -run "^TestAcc*" ./github -v -race -coverprofile=coverage.txt -covermode=atomic -sweep=tf-acc- -timeout 120m -count=1
Copy link
Contributor

Choose a reason for hiding this comment

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

question: Since this produces a coverage.txt would we benefit from uploading the artifact?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes I think so and/or integrating with a service to manage coverage.

@stevehipwell
Copy link
Collaborator Author

This is looking quite good! What would it take to get this finished?

@deiga this PR was intended to be a followup to #2476 as it only works if the tests are all actually passing. Before the PR freeze both of these PRs were working correctly and all of the tests were passing, but given the number of PRs merged since then I suspect that there may be quite a lot of changes required. I was planning on looking through the PRs and starting by taking all of the test fixes out into a new smaller and less contentious PR. The main problem here is time, both the lack of it on my side and the passage of it since I did this work that means remembering and validating the changes is going to be tricky.

@stevehipwell
Copy link
Collaborator Author

@deiga I've created #2941 to update the tests. I'll try and have a look at this in isolation but as it requires a number of GitHub repo admin changes I'd need buy in from the GitHub maintainers to make it worth continuing. I think we should take any further discussion back to #2425?

@stevehipwell
Copy link
Collaborator Author

This PR has been replaced by #2946.

@github-project-automation github-project-automation bot moved this from In Progress to Done in Terraform Provider Nov 25, 2025
@github-project-automation github-project-automation bot moved this from 👀 In review to ✅ Done in 🧰 Octokit Active Nov 25, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Development

Successfully merging this pull request may close these issues.

2 participants