Skip to content

Refactor versioning process#3714

Merged
danielrbradley merged 11 commits into
masterfrom
vendor-squeeze
Nov 19, 2024
Merged

Refactor versioning process#3714
danielrbradley merged 11 commits into
masterfrom
vendor-squeeze

Conversation

@danielrbradley

@danielrbradley danielrbradley commented Nov 18, 2024

Copy link
Copy Markdown
Contributor
  1. Copy squeeze code from schema-tools.
  2. Combine gen raw-schema + schema-tools squeeze into gen squeeze.
  3. Output removed invokes from squeeze & remove from BuildSchema.
  4. Tidy and simplify BuildSchema internal methods.
  5. Add unit tests as documentation/spec.
  6. Fix bug to get tests to pass.

Precursor to implementing #2661

@danielrbradley danielrbradley self-assigned this Nov 18, 2024
@github-actions

Copy link
Copy Markdown
Contributor

Does the PR have any schema changes?

Looking good! No breaking changes found.
No new resources/functions.

@codecov

codecov Bot commented Nov 18, 2024

Copy link
Copy Markdown

Codecov Report

Attention: Patch coverage is 18.66029% with 170 lines in your changes missing coverage. Please review.

Please upload report for BASE (master@112225d). Learn more about missing BASE report.
Report is 12 commits behind head on master.

Files with missing lines Patch % Lines
provider/pkg/squeeze/squeeze.go 17.51% 143 Missing and 3 partials ⚠️
provider/pkg/versioning/gen.go 27.77% 13 Missing ⚠️
provider/cmd/pulumi-gen-azure-native/main.go 0.00% 10 Missing ⚠️
provider/pkg/versioning/build_schema.go 0.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff            @@
##             master    #3714   +/-   ##
=========================================
  Coverage          ?   56.31%           
=========================================
  Files             ?       74           
  Lines             ?    11735           
  Branches          ?        0           
=========================================
  Hits              ?     6609           
  Misses            ?     4635           
  Partials          ?      491           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.


🚨 Try these New Features:

- Remove cobra entry point.
- Keep only CompareAll function.
- Copy isVersionedName and versionlessName from schema-tools/pkg.
- Pass schema and result via objects instead of via disk.
- Copy existing tests.
- Generate raw-schema in-memory then immediately squeeze and output the result.
- Derive filename from current major version in makefile.
- Remove schema-tools from makefile.
- Expose OpenApi providers from the BuildSchema function to avoid loading twice.
- Remove obsolete makefile dependency on the full-schema - we introduced the raw-schema to avoid this dependency.
This is no longer calculated during the BuildSchema process - only read in as it's generated at the same time as the removed resources.
- Pass openapi.AzureProviders in as a "source".
- Add documentation comments for some nested data structures.

@thomas11 thomas11 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM with nits. Great start to a proper test suite! The readability of nested subtests is a matter of taste, I guess.

Comment thread provider/pkg/versioning/defaultVersion.go Outdated
Comment thread provider/pkg/versioning/gen_calculateVersionMetadata_test.go
@danielrbradley

danielrbradley commented Nov 19, 2024

Copy link
Copy Markdown
Contributor Author

The readability of nested subtests is a matter of taste, I guess

Would you think a single level of nesting with longer test names is better instead?

Edit: flattened to a single level of nesting - just grouped by the function under test.

We incorrectly assumed that extracted keys were in order when Go actually randomises the order of keys while iterating.

- Require keys to be of an ordered type.
- Use the built-in slices.Max generic function.
Describe basic behaviour of version selection process via structured test cases.
@pulumi-bot

Copy link
Copy Markdown
Contributor

This PR has been shipped in release v2.73.0.

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