-
Notifications
You must be signed in to change notification settings - Fork 100
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
Update validate workflow to test against PR specific schema changes #7814
Conversation
8b1c8a9
to
18fa65b
Compare
7582594
to
dcd6cd6
Compare
dcd6cd6
to
7d2fe4a
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #7814 +/- ##
==========================================
- Coverage 61.06% 61.05% -0.02%
==========================================
Files 523 523
Lines 27466 27466
==========================================
- Hits 16772 16768 -4
- Misses 9210 9212 +2
- Partials 1484 1486 +2 ☔ View full report in Codecov by Sentry. |
ebe17a1
to
bf007b3
Compare
bf007b3
to
6b46ec5
Compare
6b46ec5
to
07815dc
Compare
Signed-off-by: sk593 <[email protected]>
07815dc
to
064a126
Compare
Signed-off-by: sk593 <[email protected]>
3b50c2c
to
49a0d2a
Compare
Radius functional test overview
Click here to see the list of tools in the current test run
Test Status⌛ Building Radius and pushing container images for functional tests... |
This PR helps me resolve some of the functional testing issues within #7744 |
registry-port: ${{ env.LOCAL_REGISTRY_PORT }} | ||
- name: Publish bicep types | ||
run: | | ||
bicep publish-extension ./hack/bicep-types-radius/generated/index.json --target br:${{ env.LOCAL_REGISTRY_SERVER }}:${{ env.LOCAL_REGISTRY_PORT }}/radius:latest --force |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need the registry? It is possible to place the extension on disk?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We need a registry but it doesn't have to be a locally hosted one. The action to create one already exists in the repo (and this is how we do it in other workflows) so it made sense to use that
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok cool 👍
For some reason I thought we had the ability to reference an extension as a file from bicepconfig.json
. If that were possible, it would be fewer moving pieces.
@@ -14,6 +14,7 @@ module redis '../../../test/testrecipes/modules/redis-selfhost.bicep' = { | |||
} | |||
} | |||
|
|||
#disable-next-line BCP081 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What's this one?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Resource type "dapr.io/Component@v1alpha1" does not have types available. Bicep is unable to validate resource properties prior to deployment, but this will not block the resource from being deployed.bicep(BCP081)
There's no linter rule alias for this so I used the number instead
@@ -44,7 +44,9 @@ output result object = { | |||
username: account.name | |||
} | |||
secrets: { | |||
#disable-next-line outputs-should-not-contain-secrets |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
Hi @superbeeny! The validate bicep workflow is independent from the functional tests runs so there shouldn't be overlap between the type artifacts. The functional tests should already test against PR-specific schema changes. Are you seeing issues with the functional tests not picking up schema changes? |
Radius functional test overview
Click here to see the list of tools in the current test run
Test Status⌛ Building Radius and pushing container images for functional tests... |
- name: Create a secure local registry | ||
id: create-local-registry | ||
uses: ./.github/actions/create-local-registry | ||
with: | ||
secure: "true" | ||
registry-name: ${{ env.LOCAL_REGISTRY_NAME }} | ||
registry-server: ${{ env.LOCAL_REGISTRY_SERVER }} | ||
registry-port: ${{ env.LOCAL_REGISTRY_PORT }} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The power of custom actions 💪
…adius-project#7814) # Description _Please explain the changes you've made._ ## Type of change <!-- Please select **one** of the following options that describes your change and delete the others. Clearly identifying the type of change you are making will help us review your PR faster, and is used in authoring release notes. If you are making a bug fix or functionality change to Radius and do not have an associated issue link please create one now. --> - This pull request fixes a bug in Radius and has an approved issue (issue link required). - This pull request adds or changes features of Radius and has an approved issue (issue link required). - This pull request is a minor refactor, code cleanup, test improvement, or other maintenance task and doesn't change the functionality of Radius (issue link optional). <!-- Please update the following to link the associated issue. This is required for some kinds of changes (see above). --> Fixes: #issue_number --------- Signed-off-by: sk593 <[email protected]> Signed-off-by: Reshma Abdul Rahim <[email protected]>
Description
Please explain the changes you've made.
Type of change
Fixes: #issue_number