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

test: Enables simulation of cloud-dev using hoverfly in alert configuration acceptance tests #2057

Merged
merged 21 commits into from Mar 26, 2024

Conversation

AgustinBettati
Copy link
Collaborator

@AgustinBettati AgustinBettati commented Mar 21, 2024

Description

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

How to run replay mode

A new environment variable is defined for both capturing and then simulating interactions with external APIs when running alert configuration acceptance tests.

  1. Capture requests by doing an execution of all tests in resource_alert_configuration_test.go. The following env variable must be defined:
    REPLAY_MODE=capture
    As a result of this execution you will be able to see all captured information under ./simulations folder.
  2. Simulate requests by using the following variable:
    REPLAY_MODE=simulate
    In this case the test execution will search for the specific files under ./simulations folder depending on which test is being run, and will fail if no file is available.

Implementation consideration

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 contribution guidelines
  • 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

settings.ProxyPort = fmt.Sprintf("%d", proxyPort)
hv := hoverfly.NewHoverflyWithConfiguration(settings)

if err := hv.StartProxy(); err != nil {
Copy link
Member

Choose a reason for hiding this comment

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

I think no changes needed for this PR. if we ran this in CI in many resources there will be more chances of picking an already-used port so we might want to implement a retry mechanism in the future to try in a different port until it finds one that is free

Copy link
Member

@lantoli lantoli left a comment

Choose a reason for hiding this comment

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

great job!

just some small comments

@lantoli
Copy link
Member

lantoli commented Mar 22, 2024

knitpit PR title: Enables...

@AgustinBettati AgustinBettati changed the title test: Enable simulation of cloud-dev using hoverfly in alert configuration acceptance tests test: Enables simulation of cloud-dev using hoverfly in alert configuration acceptance tests Mar 26, 2024
@AgustinBettati AgustinBettati marked this pull request as ready for review March 26, 2024 08:34
@AgustinBettati AgustinBettati requested a review from a team as a code owner March 26, 2024 08:34
var (
projectID = acc.ProjectIDExecution(t)
projectID = replay.ManageProjectID(t, func() string {
Copy link
Member

Choose a reason for hiding this comment

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

you can do: projectID = replay.ManageProjectID(t, acc.ProjectIDExecution)

if you change ManageProjectID signature:

func ManageProjectID(t *testing.T, projectIDProvider func(testing.TB) string) string {
t.Helper()
if IsInCaptureMode() {
id := projectIDProvider(t)
...

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

thanks! adjusted

Copy link
Member

@lantoli lantoli left a comment

Choose a reason for hiding this comment

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

LGTM, just a couple of minor comments

Copy link
Collaborator

@oarbusi oarbusi left a comment

Choose a reason for hiding this comment

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

Nice! LGTM

CONTRIBUTING.md Outdated

Some resources allow recording and replaying http requests using hoverfly when running tests. This is currently only configured for alert configuration resource acceptance tests.
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'd update this to say something like "for example, alert configuration..." so that we don't have to worry about updating this later when we add more.

@AgustinBettati AgustinBettati merged commit 3c68921 into master Mar 26, 2024
46 checks passed
@AgustinBettati AgustinBettati deleted the CLOUDP-237033-replay-mode branch March 26, 2024 14:15
lantoli added a commit that referenced this pull request Mar 26, 2024
* master:
  fix: Uses `overwriteBackupPolicies` in `mongodbatlas_backup_compliance_policy` to avoid overwriting non complying backup policies in updates (#2054)
  test: Enables simulation of cloud-dev using hoverfly in alert configuration acceptance tests (#2057)

# Conflicts:
#	internal/service/backupcompliancepolicy/resource_backup_compliance_policy_test.go
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.

None yet

4 participants