Skip to content

Conversation

@fmenezes
Copy link
Collaborator

@fmenezes fmenezes commented Mar 21, 2025

Proposed changes

Add snapshot testing

Jira ticket: CLOUDP-305055

Closes #[issue number]

Checklist

  • I have signed the MongoDB CLA
  • I have added tests that prove my fix is effective or that my feature works
  • I have added any necessary documentation in document requirements section listed in CONTRIBUTING.md (if appropriate)
  • I have addressed the @mongodb/docs-cloud-team comments (if appropriate)
  • I have updated test/README.md (if an e2e test has been added)
  • I have run make fmt and formatted my code

Further comments

Note: first commit has all code, second commit has a bunch of snapshots in json format

Test failures:

Excluded tests:

  • atlas,clusters,flags: needs investigation on why it fails on snapshot mode
  • atlas,interactive: needs investigation on why it fails on snapshot mode
  • atlas,deployments,atlasclusters: one assertion connects to DB (needs live mode), if we keep this assertion it will never work on snapshot mode
  • atlas,deployments,local,auth,deprecated: needs docker to run, we need to check if GA supports it
  • atlas,deployments,local,auth,new: needs docker to run, we need to check if GA supports it
  • atlas,deployments,local,nocli: needs docker to run, we need to check if GA supports it
  • atlas,deployments,local,noauth: needs docker to run, we need to check if GA supports it

Follow ups:

  • Remove tests from evergreen (deduplicate tests): not done, might be safer to monitor a couple of days first
  • Open Jira tickets when tests fail: not done needs a followup PR
  • Open Jira tickets when update job fails: not done needs a followup PR
  • Integrate e2e snapshot tests coverage calculation: not done needs a followup PR
  • Remove coverage from evergreen: depends on above item

@fmenezes fmenezes force-pushed the CLOUDP-305055 branch 3 times, most recently from 5c639d0 to 5c55578 Compare March 26, 2025 01:20
@fmenezes fmenezes marked this pull request as ready for review March 26, 2025 02:00
@fmenezes fmenezes requested a review from a team as a code owner March 26, 2025 02:00
}

for k, v := range data {
if k == "__body__" || k == "__status__" || k == "__method__" || k == "__path__" {
Copy link
Member

Choose a reason for hiding this comment

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

Could you define these strings as constants?

}

var data map[string]any
if err := json.Unmarshal(buf, &data); 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.

[q] What's the reason for using a map[string]map[string]any{} over a type Memory struct that contains a strongly typed version of the memory?

}

func updateSnapshots() bool {
return isTrue(os.Getenv("UPDATE_SNAPSHOTS"))
Copy link
Member

Choose a reason for hiding this comment

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

Could you also turn this into a constant?

jeroenvervaeke
jeroenvervaeke previously approved these changes Mar 26, 2025
@github-actions
Copy link
Contributor

Coverage Report 📈

Branch Commit Coverage
master c20ae3e 38.1%
CLOUDP-305055 56cb8db 38.1%
Difference 0%

@fmenezes fmenezes merged commit 391281c into master Mar 26, 2025
55 of 57 checks passed
@fmenezes fmenezes deleted the CLOUDP-305055 branch March 26, 2025 16:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants