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

Enable long running tests and use new federated credentials for testing #864

Merged
merged 59 commits into from
May 24, 2024

Conversation

hossam-nasr
Copy link
Contributor

This PR uses the newly created AzureDevOpsSubscriptionProvider from the most recent release of the auth package, and creates some simple CRUD E2E tests. In addition, it also:

  • Removes some references to the deprecated azure-account extension, and removes installing it as a pretest step
  • Adds webpack as a pretest step
  • Removes no-longer-needed references to web UI

@hossam-nasr hossam-nasr requested a review from a team as a code owner May 23, 2024 00:19
@@ -27,3 +27,5 @@ resources:
# Use those templates
extends:
template: azure-pipelines/1esmain.yml@azExtTemplates
parameters:
useAzureFederatedCredentials: true
Copy link
Contributor Author

Choose a reason for hiding this comment

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

If you look at the rest of the file, this pipeline file is only run when scheduled for nightly builds. Normal build pipelines that occur on PRs are done through GitHub Actions and not through this file. This is why useAzureFederatedCredentials is always set to true. For completeness sake, we could also set it to only be true if the build reason is Schedule, but that would make it harder to test manually for any reason

@@ -1,6 +1,5 @@
{
"recommendations": [
"dbaeumer.vscode-eslint",
"ms-vscode.azure-account"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

no longer need this recommendation

@@ -50,6 +50,5 @@ async function cleanReadme(): Promise<void> {

exports['webpack-dev'] = gulp.series(prepareForWebpack, () => gulp_webpack('development'));
exports['webpack-prod'] = gulp.series(prepareForWebpack, () => gulp_webpack('production'));
exports.preTest = gulp_installAzureAccount;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We no longer need to install Azure Account before tests

test/global.test.ts Outdated Show resolved Hide resolved
{
encoding: 'utf-8',
stdio: 'inherit'
});
Copy link
Contributor Author

Choose a reason for hiding this comment

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

No longer need to install Azure Account before running tests

src/extension.ts Outdated Show resolved Hide resolved
src/extension.ts Outdated Show resolved Hide resolved
nturinski
nturinski previously approved these changes May 23, 2024
@nturinski nturinski merged commit e056095 into main May 24, 2024
3 checks passed
@nturinski nturinski deleted the hossamnasr/useNewConstructor branch May 24, 2024 21:14
@microsoft microsoft locked and limited conversation to collaborators Jul 9, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants