feat: compute reservation create shared

[gryczj]

Description

Fixes #

Note: Before submitting a pull request, please open an issue for discussion if you are not associated with Google.

Checklist

• I have followed guidelines from CONTRIBUTING.MD and Samples Style Guide
• Tests pass: npm test (see Testing)
• Lint pass: npm run lint (see Style)
• These samples need a new API enabled in testing projects to pass (let us know which ones)
• These samples need a new/updated env vars in testing projects set to pass (let us know which ones)
• This pull request is from a branch created directly off of GoogleCloudPlatform/nodejs-docs-samples. Not a fork.
• This sample adds a new sample directory, and I updated the CODEOWNERS file with the codeowners for this sample
• This sample adds a new sample directory, and I created GitHub Actions workflow for this sample
• This sample adds a new Product API, and I updated the Blunderbuss issue/PR auto-assigner with the codeowners for this sample
• Please merge this PR for me once it is approved

[snippet-bot]

Here is the summary of changes.

You are about to add 1 region tag.

• compute/reservations/createSharedReservation.js:20, tag compute_reservation_create_shared

---

This comment is generated by snippet-bot.
If you find problems with this result, please file an issue at:
https://github.com/googleapis/repo-automation-bots/issues.
To update this comment, add snippet-bot:force-run label or use the checkbox below:

• Refresh this comment

[gryczj]

Please review #3868 first

[gryczj]

Reason for: /* eslint-disable no-dupe-keys */
According to the documentation: https://cloud.google.com/compute/docs/instances/reservations-modify#modifying-consumer-projects-shared-reservation. When we want to update consumer projects, we must define paths field for each consumer, e.g:

await reservationsClient.update({
      project: projectId,
      reservation: reservationName,
      paths: 'shareSettings.projectMap.consumerId1',
      paths: 'shareSettings.projectMap.consumerId2'
      zone,
      reservationResource: {
        name: reservationName,
        shareSettings: {
          projectMap: {
            consumerId1: {
              projectId: 'consumerId1',
            },
            consumerId2: {
              projectId: 'consumerId2',
            },
          },
        },
      },
    });

[subfuzion]

Really nice use of mocks, but overkill for a sample. Please simplify, per the comment I left. Thanks!

[subfuzion]

Hi @gryczj. My comment here will apply to both modules and to the test. What really are we looking for to know if the sample did what it was intended? We are not unit testing production APIs, we simply expect that our call to an API will have the intended result. What is that? A resource ID? Not returning a failed status code or catching an exception, etc? If you were a developer using this API in production code, what would you expect -- that's the check you could demonstrate right here. The unit test probably doesn't really need mocks and all the extra complexity; isn't it enough, really, if main doesn't throw (the process doesn't exit with a non-zero exit code)? Our samples shouldn't themselves be APIs, so no need to export anything out of this module.

[subfuzion]

I just want to add your PRs have been great! To clarify here, the goal is that a sample should fully encapsulate the demonstration of what a developer needs to know to use our APIs successfully in their own production code. How is the API intended to be used? Our tests don't need to confirm that APIs work as expected, only that the sample works (we don't want broken samples in our docs for an unexpected reason, but we do expect that service teams have already exhaustively tested their services). Hope this helps!

[gryczj]

@subfuzion thank you for your comments and review :) I introduced mocks here because to create shared reservation special permission are required and also another project, that we want to use as consumer and share this reservation with. I can test this sample locally with my credentials, but I have no knowledge about our test environment and projects, which I can use here as an example in tests and this sample will throw errors.
I understand your point. All new changes in the implementation were made to allow me to use mocks and fake clients in tests and on the other hand allow user to use real clients if they want to.

[m-strzelczyk]

@subfuzion As @gryczj wrote, the shared reservation sample would require a separate project, with a change to organization policy to allow this project to share the reservation with other projects.

I made an "executive" call for the team that writes those samples for Python, Java, Go and Node to use mocks for those samples as a kind of an exception to the rule.

My arguments:

• We don't really do tests that span multiple projects.
• It'd be very easy to forget about this additional project and in a year or two we'd have to rediscover the whole thing.
• I know that Python has separate projects for every version of the language - having tests that span multiple projects that require org policy updates would make it harder to start up a new project for new version.

We acknowledge the value of proper testing and that's why mocks were picked as a middle ground between skipping tests for those samples and writing fully functional tests that would make use of multiple projects.

Let me know if I managed to convince you or not :)

[subfuzion]

Thanks for the added context. I still don't see the need to mock console or treat the sample like its library code and have it export a function for testing, and my points about overall test simplification still remain. I'd like to surface this up to the rest of our team for discussion, so can you please address the points I've raised specifically?

Also, overall test flakiness still seems to be a problem, such as the compute hyperdisk failures for #3834 (which I merged so we can consolidate efforts on a single PR).

Thanks!

[subfuzion]

Please feel free to ping me directly on internal chat: tonypujals@

[gryczj]

To keep commit history clean I moved changes to new PR: #3894