Skip to content

Conversation

manupedrozo
Copy link
Collaborator

@manupedrozo manupedrozo commented Oct 6, 2025

Description

Add acceptance tests for project_settings_api

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

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. A migration guide must be created or updated if the new feature will go in a major version.
  • Breaking change (fix or feature that would cause existing functionality to not work as expected). Please, add the "breaking change" label to the PR. A migration guide must be created or updated.
  • 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 have added appropriate changelog entries.
  • 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

@manupedrozo manupedrozo marked this pull request as ready for review October 6, 2025 12:24
@manupedrozo manupedrozo requested a review from a team as a code owner October 6, 2025 12:24
group_id = %[1]q
is_collect_database_specifics_statistics_enabled = %[2]t
is_data_explorer_enabled = %[2]t
is_data_explorer_gen_ai_features_enabled = %[2]t
Copy link
Member

Choose a reason for hiding this comment

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

q: Noticed this is the only attribute that is optional-only, does the API behave differently or we simply missed this additional config?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Good catch, adding config for this one.

Comment on lines +32 to +35
{
Config: config(projectID, false),
Check: check(projectID, false),
},
Copy link
Member

Choose a reason for hiding this comment

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

nit: Could be could to add a step where some or all attributes are not defined, given they are O+C values in state would not change.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

👌 Added an empty config step.

is_data_explorer_gen_ai_features_enabled = %[2]t
is_data_explorer_gen_ai_sample_document_passing_enabled = %[2]t
is_extended_storage_sizes_enabled = %[2]t
is_performance_advisor_enabled = %[2]t
Copy link
Member

Choose a reason for hiding this comment

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

nit: passing the same value to attributes could hide some assignment errors (e.g. we put in is_performance_advisor_enabled the value of is_extended_storage_sizes_enabled)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I considered this but decided against testing each value individually as of now.
How do we approach this typically? For the handwritten project resource we are not testing individual assignments so probably not worth doing it here either but open to suggestions.


check := resource.ComposeAggregateTestCheckFunc(
checkExists(resourceName),
resource.TestCheckResourceAttr(resourceName, "group_id", projectID),
Copy link
Member

@lantoli lantoli Oct 6, 2025

Choose a reason for hiding this comment

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

consider using some of the helper functions we have to group test checks, e.g. AddAttrChecks and similar

Copy link
Collaborator Author

@manupedrozo manupedrozo Oct 6, 2025

Choose a reason for hiding this comment

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

Done 6d0aae5

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants