doc: Documents testing best practices #2132

lantoli · 2024-04-10T06:57:04Z

Description

Documents testing best practices

Link to any related issue(s): CLOUDP-242165

Type of change:

Bug fix (non-breaking change which fixes an issue). Please, add the "bug" label to the PR.
New feature (non-breaking change which adds functionality). Please, add the "enhancement" label to the PR.
Breaking change (fix or feature that would cause existing functionality to not work as expected). Please, add the "breaking change" label to the PR.
This change requires a documentation update
Documentation fix/enhancement

Required Checklist:

I have signed the MongoDB CLA
I have read the contributing guides
I have checked that this change does not generate any credentials and that they are NOT accidentally logged anywhere.
I have added tests that prove my fix is effective or that my feature works per HashiCorp requirements
I have added any necessary documentation (if appropriate)
I have run make fmt and formatted my code
If changes include deprecations or removals, I defined an isolated PR with a relevant title as it will be used in the auto-generated changelog.
If changes include removal or addition of 3rd party GitHub actions, I updated our internal document. Reach out to the APIx Integration slack channel to get access to the internal document.

Further comments

contributing/testing-best-practices.md

AgustinBettati

LGTM, great to have this in place. Left minor suggestions.

contributing/testing-best-practices.md

marcosuma · 2024-04-10T09:54:56Z

contributing/testing-best-practices.md

@@ -0,0 +1,49 @@
+
+# Testing Best Practices


comment for the different sections so leaving it here. Can you add examples to our current tests when describing the guidelines? You can even pick a resource and always refer to it

do you mean for instance when describing variable MONGODB_ATLAS_PROJECT_ID, function ProjectIDExecution, etc. to link to some part in the code?

if that's the case I'm not sure I would do that because it's easy that links to code get outdated when we change the code/tests.

I think it's better that people just search for words like MONGODB_ATLAS_PROJECT_ID in the code to see how it can be used.

I left a couple of cases below of where to leave an example.

it's easy that links to code get outdated when we change the code/tests.
Not properly, you can make a link to a file by fixing the commit. Also if an example ever gets outdated it means also the guideline is outdated, so it doesn't really create an extra effort.

Rationale is to think: how can I speed up my team to implement the right practices and "see" what I mean?

adding examples

marcosuma · 2024-04-10T09:59:19Z

contributing/testing-best-practices.md

+
+- There must be at least one `basic acceptance test` for each resource
+- `Basic import tests` are done as the last step in the `basic acceptance tests`, not as a different test. Exceptions apply for more specific import tests, e.g. testing with incorrect IDs.
+- Data sources are tested in the same tests as the resources. There are not specific test files or tests for data sources as a resource must typically be created. (There are very few exceptions to this, e.g. when there is only data sources but not resource)


add example.

Suggested change

- Data sources are tested in the same tests as the resources. There are not specific test files or tests for data sources as a resource must typically be created. (There are very few exceptions to this, e.g. when there is only data sources but not resource)

- Data sources are tested in the same tests as the resources. There are no separate test files or tests for data sources as a resource must typically be created. (There are very few exceptions to this, e.g. when there is only data sources but not resource)

similar comment to above. if we have links to specific parts of the code/test i think it's easy that they get outdated. or maybe I could just reference the file not specific lines

I believe you can use permalink, e.g.

terraform-provider-mongodbatlas/internal/service/advancedcluster/model_advanced_cluster_test.go

Line 24 in 66c44e6

type Result struct {

-- this link will always refer to the same line, even when the file evolves because it points to a specific commit

yes, but that would be a link to a previous version of the file that might differ significantly from the current file. Maybe the specific line(s) are ok but the rest of the file might have content that is outdated and got improvements in the latest version. user might be confused to think that the whole file (not only the highlighted lines) has the latest best-practices. file might not even exist anymore

adding examples

contributing/testing-best-practices.md

Co-authored-by: maastha <[email protected]>

Co-authored-by: Agustin Bettati <[email protected]>

…SetElemAttr`

contributing/testing-best-practices.md

gssbzn · 2024-04-11T11:30:52Z

contributing/testing-best-practices.md

+
+- There must be at least one `basic acceptance test` for each resource.
+- `Basic import tests` are done as the last step in the `basic acceptance tests`, not as a different test. Exceptions apply for more specific import tests, e.g. testing with incorrect IDs. [Import tests](https://developer.hashicorp.com/terraform/plugin/sdkv2/resources/import#resource-acceptance-testing-implementation) verify that the [Terraform Import](https://developer.hashicorp.com/terraform/cli/import) functionality is working fine.
+- Data sources are tested in the same tests as the resources. There are no separate files or tests for data sources as a resource must typically be created. (There are very few exceptions to this, e.g. when there is only data sources but not resource)


this seems to belong to test organization

also atlas_user for example doesn't have resource

contributing/testing-best-practices.md

gssbzn · 2024-04-11T11:37:26Z

contributing/testing-best-practices.md

+
+Acceptance and migration tests can reuse projects and clusters in order to be more efficient in resource utilization.
+
+- A project can be reused using `ProjectIDExecution`. It returns the ID of a project that is created for the current execution of tests for a resource.


reading usage seems to be

Suggested change

- A project can be reused using `ProjectIDExecution`. It returns the ID of a project that is created for the current execution of tests for a resource.

- A project can be reused using `acc.ProjectIDExecution(t)`. It returns the ID of a project that is created for the current execution of tests for a resource.

But I would suggest this may be better explained with a code block that includes the imports and comments on all the bits needed,

adding example

contributing/testing-best-practices.md

marcosuma

LGTM! I think it's also a type of documentation that is live and will always be improved. Make sure to wait also for Gus'👍

contributing/testing-best-practices.md

marcosuma · 2024-04-12T14:40:40Z

contributing/testing-best-practices.md

+- Migration (mig) tests: These tests are designed to ensure that after an upgrade to a new Atlas provider version, user configs do not result in unexpected plan changes, more info [here](https://developer.hashicorp.com/terraform/plugin/framework/migrating/testing). Migration tests are a subset of Acceptance tests.
+
+## File structure
+- A resource and associated data sources are implemented in a folder that is also a Go package, e.g. `advancedcluster` implementation is in [`internal/service/advancedcluster`](https://github.com/mongodb/terraform-provider-mongodbatlas/tree/master/internal/service/advancedcluster)


Suggested change

- A resource and associated data sources are implemented in a folder that is also a Go package, e.g. `advancedcluster` implementation is in [`internal/service/advancedcluster`](https://github.com/mongodb/terraform-provider-mongodbatlas/tree/master/internal/service/advancedcluster)

- A resource and associated data sources are implemented in a folder that is also a Go package, e.g. `advancedcluster` implementation is in [`internal/service/advancedcluster`](https://github.com/mongodb/terraform-provider-mongodbatlas/tree/f3ff5bb678c1b07c16cc467471f483e483565427/internal/service/advancedcluster)

feel free to discard if there was a rationale for non using permalink, I think it's better though

not a strong opinion, for links that are that not probably going to change like this one (resource folder) i might use master, and from that link you can navigate through the latest advanced_cluster code

similar for main_test.go, if the link to that file gets broken it's really because we've changed the convention and we should have updated the doc

thanks @gssbzn for all your comments. as discussed offline i'll merge the PR

Co-authored-by: Marco Suma <[email protected]>

EspenAlbert

Great job 👏

Co-authored-by: Gustavo Bazan <[email protected]>

* master: fix: Removes default comment when creating resource `mongodbatlas_privatelink_endpoint_serverless` and adds update support to `mongodbatlas_privatelink_endpoint_service_serverless` (#2133) doc: Documents testing best practices (#2132) chore: Uses correct format when setting remote (#2147) chore: Configures git correctly for automatic commit (#2146) chore: Uses PAT of bot to commit changelog updates (#2144) doc: Adjust RELEASING.md documentation with new process (#2142) fix: Fixes nil pointer dereference if `advanced_configuration` update fails in `mongodbatlas_cluster` (#2139) chore: Adds a new step in release process that marks Jira version as released (#2136) chore: Sends Slack notification when changelog update fails (#2140) chore: Adds new step in release process for updating header of changelog (#2134)

lantoli added 6 commits April 10, 2024 08:55

split development and testing contributing guides

31f2b9f

test types

60cdb0f

local development

21b927e

acc and mig tests

06670dd

periods

b9aeb28

hoverfly in local

24226c0

lantoli marked this pull request as ready for review April 10, 2024 08:57

lantoli requested a review from a team as a code owner April 10, 2024 08:57

gssbzn reviewed Apr 10, 2024

View reviewed changes

contributing/testing-best-practices.md Outdated Show resolved Hide resolved

fix section headers

f9beae5

AgustinBettati approved these changes Apr 10, 2024

View reviewed changes

contributing/testing-best-practices.md Outdated Show resolved Hide resolved

contributing/testing-best-practices.md Outdated Show resolved Hide resolved

maastha reviewed Apr 10, 2024

View reviewed changes

contributing/testing-best-practices.md Outdated Show resolved Hide resolved

maastha reviewed Apr 10, 2024

View reviewed changes

contributing/testing-best-practices.md Outdated Show resolved Hide resolved

EspenAlbert reviewed Apr 10, 2024

View reviewed changes

contributing/testing-best-practices.md Outdated Show resolved Hide resolved

EspenAlbert reviewed Apr 10, 2024

View reviewed changes

contributing/testing-best-practices.md Outdated Show resolved Hide resolved

EspenAlbert reviewed Apr 10, 2024

View reviewed changes

contributing/testing-best-practices.md Outdated Show resolved Hide resolved

EspenAlbert reviewed Apr 10, 2024

View reviewed changes

contributing/testing-best-practices.md Outdated Show resolved Hide resolved

marcosuma reviewed Apr 10, 2024

View reviewed changes

EspenAlbert reviewed Apr 10, 2024

View reviewed changes

contributing/testing-best-practices.md Outdated Show resolved Hide resolved

lantoli and others added 11 commits April 10, 2024 12:08

Update contributing/testing-best-practices.md

f51edd7

Co-authored-by: maastha <[email protected]>

Update contributing/testing-best-practices.md

17ca19d

Co-authored-by: maastha <[email protected]>

Update contributing/testing-best-practices.md

d1870b4

Co-authored-by: Agustin Bettati <[email protected]>

typo

da20a1f

extract Shared resources section

2599b3a

table for local development

eb26202

resource.TestCheckTypeSetElemNestedAttrs or `resource.TestCheckType…

833db9b

…SetElemAttr`

use must

13ae246

rephase separate

e677f7b

links to acc & mig tests

b4a9de6

note about mocks

09d9362

lantoli added 2 commits April 11, 2024 12:55

clarification added

b3263a8

explain import tests

ff3603e

gssbzn reviewed Apr 11, 2024

View reviewed changes

marcosuma reviewed Apr 11, 2024

View reviewed changes

contributing/testing-best-practices.md Outdated Show resolved Hide resolved

lantoli added 7 commits April 12, 2024 13:41

rephrase plural ds

2229962

change section name Local testing

9dd870f

explain basic tests

b4d1118

apply feedback

5f47775

more feedback

54a676b

links

a1cbcbe

examples

6916f9b

marcosuma approved these changes Apr 12, 2024

View reviewed changes

Update contributing/testing-best-practices.md

3ea7551

Co-authored-by: Marco Suma <[email protected]>

EspenAlbert approved these changes Apr 12, 2024

View reviewed changes

lantoli and others added 2 commits April 12, 2024 17:36

clarify common functions

feae31a

Update contributing/testing-best-practices.md

94735af

Co-authored-by: Gustavo Bazan <[email protected]>

maastha approved these changes Apr 12, 2024

View reviewed changes

lantoli merged commit 5204df2 into master Apr 12, 2024
47 checks passed

lantoli deleted the CLOUDP-242165_doc_testing branch April 12, 2024 16:14

This was referenced Apr 17, 2024

chore: Adds testpackage linter #2164

Merged

chore: Fixes markdown sections #2165

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

doc: Documents testing best practices #2132

doc: Documents testing best practices #2132

lantoli commented Apr 10, 2024 •

edited

Loading

AgustinBettati left a comment

marcosuma Apr 10, 2024

lantoli Apr 11, 2024

marcosuma Apr 11, 2024

lantoli Apr 12, 2024

marcosuma Apr 10, 2024

lantoli Apr 11, 2024

marcosuma Apr 11, 2024

lantoli Apr 11, 2024 •

edited

Loading

lantoli Apr 12, 2024

gssbzn Apr 11, 2024

lantoli Apr 12, 2024

gssbzn Apr 11, 2024

lantoli Apr 12, 2024

marcosuma left a comment

marcosuma Apr 12, 2024

marcosuma Apr 12, 2024

lantoli Apr 12, 2024 •

edited

Loading

lantoli Apr 12, 2024

lantoli Apr 12, 2024

EspenAlbert left a comment

	- Data sources are tested in the same tests as the resources. There are not specific test files or tests for data sources as a resource must typically be created. (There are very few exceptions to this, e.g. when there is only data sources but not resource)
	- Data sources are tested in the same tests as the resources. There are no separate test files or tests for data sources as a resource must typically be created. (There are very few exceptions to this, e.g. when there is only data sources but not resource)


		Acceptance and migration tests can reuse projects and clusters in order to be more efficient in resource utilization.

		- A project can be reused using `ProjectIDExecution`. It returns the ID of a project that is created for the current execution of tests for a resource.

	- A project can be reused using `ProjectIDExecution`. It returns the ID of a project that is created for the current execution of tests for a resource.
	- A project can be reused using `acc.ProjectIDExecution(t)`. It returns the ID of a project that is created for the current execution of tests for a resource.

	- A resource and associated data sources are implemented in a folder that is also a Go package, e.g. `advancedcluster` implementation is in [`internal/service/advancedcluster`](https://github.com/mongodb/terraform-provider-mongodbatlas/tree/master/internal/service/advancedcluster)
	- A resource and associated data sources are implemented in a folder that is also a Go package, e.g. `advancedcluster` implementation is in [`internal/service/advancedcluster`](https://github.com/mongodb/terraform-provider-mongodbatlas/tree/f3ff5bb678c1b07c16cc467471f483e483565427/internal/service/advancedcluster)

doc: Documents testing best practices #2132

doc: Documents testing best practices #2132

Conversation

lantoli commented Apr 10, 2024 • edited Loading

Description

Type of change:

Required Checklist:

Further comments

AgustinBettati left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lantoli Apr 11, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

marcosuma left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lantoli Apr 12, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

EspenAlbert left a comment

Choose a reason for hiding this comment

lantoli commented Apr 10, 2024 •

edited

Loading

lantoli Apr 11, 2024 •

edited

Loading

lantoli Apr 12, 2024 •

edited

Loading