Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 27 additions & 3 deletions packages/schematics/angular/vitest-browser/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ import {
} from '../utility/dependency';
import { JSONFile } from '../utility/json-file';
import { latestVersions } from '../utility/latest-versions';
import { getWorkspace } from '../utility/workspace';
import { getWorkspace, updateWorkspace } from '../utility/workspace';
import { Builders } from '../utility/workspace-models';
import { Schema as VitestBrowserOptions } from './schema';

Expand Down Expand Up @@ -89,8 +89,33 @@ export default function (options: VitestBrowserOptions): Rule {
}
};

// Determine the default browser based on the provider package
let defaultBrowser: string;
if (packageName === '@vitest/browser-webdriverio') {
defaultBrowser = 'chrome';
} else {
// Playwright and preview both use 'chromium' as the default
defaultBrowser = 'chromium';
}
Comment on lines +93 to +99
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

For better code clarity and to prevent accidental reassignment, you can declare defaultBrowser as a const and initialize it using a ternary operator.

    // Playwright and preview both use 'chromium' as the default
    const defaultBrowser = packageName === '@vitest/browser-webdriverio' ? 'chrome' : 'chromium';


// Update angular.json to add the browsers option to the test target
const updateAngularJsonRule = updateWorkspace((workspace) => {
const project = workspace.projects.get(options.project);
if (project) {
const testTarget = project.targets.get('test');
if (testTarget) {
testTarget.options ??= {};
const existingBrowsers = testTarget.options['browsers'] as string[] | undefined;
if (!existingBrowsers?.length) {
testTarget.options['browsers'] = [defaultBrowser];
}
}
}
Comment on lines +103 to +113
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

The nested if checks for project and testTarget are redundant because their existence is already validated at the start of the schematic (lines 31-43), where an exception is thrown if they are not found. You can simplify this code to improve readability, as explicit checks are redundant when upstream processes guarantee existence.

      const project = workspace.projects.get(options.project);
      const testTarget = project?.targets.get('test');

      if (testTarget) {
        testTarget.options ??= {};
        const existingBrowsers = testTarget.options['browsers'] as string[] | undefined;
        if (!existingBrowsers?.length) {
          testTarget.options['browsers'] = [defaultBrowser];
        }
      }
References
  1. When refactoring code that handles configuration properties, explicit checks can be removed if the property is guaranteed to be valid by an upstream process.

});

return chain([
updateTsConfigRule,
updateAngularJsonRule,
...dependencies.map((name) =>
addDependency(name, latestVersions[name], {
type: DependencyType.Dev,
Expand All @@ -101,8 +126,7 @@ export default function (options: VitestBrowserOptions): Rule {
(_, context) => {
context.logger.info(
'Vitest browser testing support has been added. ' +
"To run tests in a browser, add a 'browsers' field to the 'test' target in 'angular.json', " +
"or use the '--browsers' command line option.",
`The test target has been configured with '${defaultBrowser}' as the default browser.`,
);
},
]);
Expand Down
53 changes: 53 additions & 0 deletions packages/schematics/angular/vitest-browser/index_spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,59 @@ describe('Vitest Browser Provider Schematic', () => {
expect(tsConfig.compilerOptions.types).not.toContain('jasmine');
});

it('should add browsers option to angular.json for playwright', async () => {
const options = {
project: 'app',
package: '@vitest/browser-playwright',
skipInstall: true,
};

const resultTree = await schematicRunner.runSchematic('vitest-browser', options, tree);

const angularJson = parse(resultTree.readContent('/angular.json'));
const project = angularJson.projects.app;
const targets = project.architect || project.targets;
expect(targets.test.options.browsers).toEqual(['chromium']);
});

it('should add browsers option to angular.json for webdriverio', async () => {
const options = {
project: 'app',
package: '@vitest/browser-webdriverio',
skipInstall: true,
};

const resultTree = await schematicRunner.runSchematic('vitest-browser', options, tree);

const angularJson = parse(resultTree.readContent('/angular.json'));
const project = angularJson.projects.app;
const targets = project.architect || project.targets;
expect(targets.test.options.browsers).toEqual(['chrome']);
});

it('should not overwrite existing browsers option in angular.json', async () => {
// Set up existing browsers option
const angularJson = parse(tree.readContent('/angular.json'));
const project = angularJson.projects.app;
const targets = project.architect || project.targets;
targets.test.options ??= {};
targets.test.options.browsers = ['firefox'];
tree.overwrite('/angular.json', JSON.stringify(angularJson));

const options = {
project: 'app',
package: '@vitest/browser-playwright',
skipInstall: true,
};

const resultTree = await schematicRunner.runSchematic('vitest-browser', options, tree);

const updatedAngularJson = parse(resultTree.readContent('/angular.json'));
const updatedProject = updatedAngularJson.projects.app;
const updatedTargets = updatedProject.architect || updatedProject.targets;
expect(updatedTargets.test.options.browsers).toEqual(['firefox']);
});

it('should add webdriverio dependency when @vitest/browser-webdriverio is used', async () => {
const options = {
project: 'app',
Expand Down
Loading