Skip to content

Make acceptance tests internal#2786

Merged
EmilienM merged 2 commits intogophercloud:masterfrom
shiftstack:tests-internal
Sep 22, 2023
Merged

Make acceptance tests internal#2786
EmilienM merged 2 commits intogophercloud:masterfrom
shiftstack:tests-internal

Conversation

@mandre
Copy link
Copy Markdown
Contributor

@mandre mandre commented Sep 22, 2023

Do not expose acceptance tests packages externally. This has the added benefit of go-apidiff no longer reporting false positive when analyzing for backward incompatible changes.

@github-actions github-actions bot added the semver:major Breaking change label Sep 22, 2023
@mandre
Make acceptance tests internal
6f31881 
Do not expose acceptance tests packages externally. This has the added
benefit of go-apidiff no longer reporting false positive when analyzing
for backward incompatible changes.
@mandre
Fix acceptancetest script when running without argument
7bec78e 
It previously generated an error due to the `ACCEPTANCE_TESTS` variable
being undefined. It now runs all acceptance tests.
@EmilienM EmilienM added backport-v1 This PR will be backported to v1 semver:patch No API change and removed semver:major Breaking change labels Sep 22, 2023
kayrus
kayrus approved these changes Sep 22, 2023
View reviewed changes
Copy link
Copy Markdown
Contributor

@kayrus kayrus left a comment

Choose a reason for hiding this comment

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

Nice decision, thanks!

@github-actions github-actions bot added semver:major Breaking change and removed semver:patch No API change labels Sep 22, 2023
@EmilienM EmilienM added semver:patch No API change and removed semver:major Breaking change labels Sep 22, 2023
@coveralls
Copy link
Copy Markdown

coveralls commented Sep 22, 2023

Coverage Status

coverage: 77.415%. remained the same when pulling 7bec78e on shiftstack:tests-internal into d77fd1a on gophercloud:master.

@EmilienM EmilienM merged commit 502a173 into gophercloud:master Sep 22, 2023
@EmilienM EmilienM deleted the tests-internal branch September 22, 2023 18:18
@gophercloud-backport-bot gophercloud-backport-bot bot mentioned this pull request Sep 22, 2023
Merged
@SuperSandro2000
Copy link
Copy Markdown
Contributor

SuperSandro2000 commented Sep 28, 2023

We are extending the acceptance tests for our sap specific additions from some of those packages. Could those be moved back?

Specifically github.com/gophercloud/gophercloud/acceptance/clients https://github.com/sapcc/gophercloud-sapcc/blob/master/acceptance/automation/v1/client.go#L22 and github.com/gophercloud/gophercloud/acceptance/tools https://github.com/sapcc/gophercloud-sapcc/blob/master/acceptance/automation/v1/automations.go#L21

@SuperSandro2000 SuperSandro2000 mentioned this pull request Sep 28, 2023
Closed
1 task
@pierreprinetti
Copy link
Copy Markdown
Member

pierreprinetti commented Sep 29, 2023

We are extending the acceptance tests for our sap specific additions from some of those packages. Could those be moved back?

Specifically github.com/gophercloud/gophercloud/acceptance/clients https://github.com/sapcc/gophercloud-sapcc/blob/master/acceptance/automation/v1/client.go#L22 and github.com/gophercloud/gophercloud/acceptance/tools https://github.com/sapcc/gophercloud-sapcc/blob/master/acceptance/automation/v1/automations.go#L21

If I understand your case correctly, you maintain a domain-specific extension of Gophercloud; to test it, you extend Gophercloud's acceptance tests in parallel. Is that what's happening here?

What we want is to remove the acceptance tests from the API surface of Gophercloud. Moving the packages to an /internal directory is just the way we've gone to achieve that.

One alternative to using /internal packages would be to move all acceptance-related code to either _test.go files or to testing packages. @SuperSandro2000 would this solution fit your case?

@mandre mandre mentioned this pull request Oct 9, 2023
Merged
@SuperSandro2000
Copy link
Copy Markdown
Contributor

SuperSandro2000 commented Oct 10, 2023

If I understand your case correctly, you maintain a domain-specific extension of Gophercloud; to test it, you extend Gophercloud's acceptance tests in parallel. Is that what's happening here?

Yes

would this solution fit your case?

Yeah, that should work, too.

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

Reviewers

@EmilienM EmilienM EmilienM approved these changes

@kayrus kayrus kayrus approved these changes

Assignees

No one assigned

Labels

backport-v1 This PR will be backported to v1 semver:patch No API change

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@mandre @coveralls @SuperSandro2000 @pierreprinetti @EmilienM @kayrus