Improve CLI & CLI Testing (#2245)

* Fix astro --version, astro --help

* Improve CLI testing

* nit: fix for windows

* nit: try different async cli tests

* fix: core dev

* nit: cleanup core

* nit: change port for config test

* nit: write config differently than project-root

* nit: cleanup AstroDevServer properties

packages/astro/src/cli/index.ts

@@ -64,7 +64,7 @@ function resolveArgs(flags: Arguments): CLIState {
 
 /** Display --help flag */
 function printHelp() {
-  console.error(`  ${colors.bold('astro')} - Futuristic web development tool.
+  console.log(`  ${colors.bold('astro')} - Futuristic web development tool.
   ${colors.bold('Commands:')}
   astro dev             Run Astro in development mode.
   astro build           Build a pre-compiled production version of your site.
@@ -85,8 +85,11 @@ function printHelp() {
 
 /** Display --version flag */
 async function printVersion() {
-  const pkg = JSON.parse(await fs.promises.readFile(new URL('../package.json', import.meta.url), 'utf8'));
-  console.error(pkg.version);
+  const pkgURL = new URL('../../package.json', import.meta.url);
+  const pkg = JSON.parse(await fs.promises.readFile(pkgURL, 'utf8'));
+  const pkgVersion = pkg.version;
+
+  console.log(pkgVersion);
 }
 
 /** Merge CLI flags & config options (CLI flags take priority) */
@@ -105,11 +108,21 @@ export async function cli(args: string[]) {
   const options = { ...state.options };
   const projectRoot = options.projectRoot || flags._[3];
 
+  switch (state.cmd) {
+    case 'help':
+      printHelp();
+      return process.exit(0);
+    case 'version':
+      await printVersion();
+      return process.exit(0);
+  }
+
   // logLevel
   let logging: LogOptions = {
     dest: defaultLogDestination,
     level: 'info',
   };
+
   if (flags.verbose) logging.level = 'debug';
   if (flags.silent) logging.level = 'silent';
   let config: AstroConfig;
@@ -118,7 +131,7 @@ export async function cli(args: string[]) {
     mergeCLIFlags(config, options);
   } catch (err) {
     if (err instanceof z.ZodError) {
-      console.log(formatConfigError(err));
+      console.error(formatConfigError(err));
     } else {
       console.error(colors.red((err as any).toString() || err));
     }
@@ -126,23 +139,15 @@ export async function cli(args: string[]) {
   }
 
   switch (state.cmd) {
-    case 'help': {
-      printHelp();
-      process.exit(1);
-      return;
-    }
-    case 'version': {
-      await printVersion();
-      process.exit(0);
-      return;
-    }
     case 'dev': {
       try {
-        const server = await devServer(config, { logging });
+        await devServer(config, { logging });
+
         await new Promise(() => {}); // don’t close dev server
       } catch (err) {
         throwAndExit(err);
       }
+
       return;
     }
     case 'build': {
@@ -156,8 +161,7 @@ export async function cli(args: string[]) {
     }
     case 'check': {
       const ret = await check(config);
-      process.exit(ret);
-      return;
+      return process.exit(ret);
     }
     case 'preview': {
       try {

packages/astro/src/core/dev/index.ts

@@ -57,27 +57,27 @@ export default async function dev(config: AstroConfig, options: DevOptions = { l
 
 /** Dev server */
 export class AstroDevServer {
-  app = connect();
-  httpServer: http.Server | undefined;
+  app: connect.Server = connect();
+  config: AstroConfig;
+  devRoot: string;
   hostname: string;
+  httpServer: http.Server | undefined;
+  logging: LogOptions;
+  manifest: ManifestData;
+  mostRecentRoute?: RouteData;
+  origin: string;
   port: number;
-  private config: AstroConfig;
-  private logging: LogOptions;
-  private manifest: ManifestData;
-  private mostRecentRoute?: RouteData;
-  private site: URL | undefined;
-  private devRoot: string;
-  private url: URL;
-  private origin: string;
-  private routeCache: RouteCache = {};
-  private viteServer: vite.ViteDevServer | undefined;
+  routeCache: RouteCache = {};
+  site: URL | undefined;
+  url: URL;
+  viteServer: vite.ViteDevServer | undefined;
 
   constructor(config: AstroConfig, options: DevOptions) {
     this.config = config;
     this.hostname = config.devOptions.hostname || 'localhost';
     this.logging = options.logging;
     this.port = config.devOptions.port;
-    this.origin = `http://localhost:${this.port}`;
+    this.origin = `http://${this.hostname}:${this.port}`;
     this.site = config.buildOptions.site ? new URL(config.buildOptions.site) : undefined;
     this.devRoot = this.site ? this.site.pathname : '/';
     this.url = new URL(this.devRoot, this.origin);
@@ -204,7 +204,7 @@ export class AstroDevServer {
   public listen(devStart: number): Promise<void> {
     let showedPortTakenMsg = false;
     return new Promise<void>((resolve, reject) => {
-      const listen = () => {
+      const appListen = () => {
         this.httpServer = this.app.listen(this.port, this.hostname, () => {
           info(this.logging, 'astro', msg.devStart({ startupTime: performance.now() - devStart }));
           info(this.logging, 'astro', msg.devHost({ host: `http://${this.hostname}:${this.port}${this.devRoot}` }));
@@ -220,7 +220,7 @@ export class AstroDevServer {
             showedPortTakenMsg = true; // only print this once
           }
           this.port++;
-          return listen(); // retry
+          return appListen(); // retry
         } else {
           error(this.logging, 'astro', err.stack);
           this.httpServer?.removeListener('error', onError);
@@ -228,7 +228,7 @@ export class AstroDevServer {
         }
       };
 
-      listen();
+      appListen();
     });
   }
 

packages/astro/src/core/preview/index.ts

@@ -38,8 +38,8 @@ export default async function preview(config: AstroConfig, { logging }: PreviewO
     }).pipe(res);
   });
 
-  let port = config.devOptions.port;
-  const { hostname } = config.devOptions;
+  let { hostname, port } = config.devOptions;
+
   let httpServer: http.Server;
 
   /** Expose dev server to `port` */

packages/astro/test/cli.test.js

@@ -0,0 +1,47 @@
+import { expect } from 'chai';
+import { cli } from './test-utils.js';
+import { promises as fs } from 'fs';
+import { fileURLToPath } from 'url';
+
+describe('astro cli', () => {
+  it('astro', async () => {
+    const proc = await cli();
+
+    expect(proc.stdout).to.include('astro - Futuristic web development tool');
+  });
+
+  it('astro --version', async () => {
+    const pkgURL = new URL('../package.json', import.meta.url);
+    const pkgVersion = await fs.readFile(pkgURL, 'utf8').then((data) => JSON.parse(data).version);
+
+    const proc = await cli('--version');
+
+    expect(proc.stdout).to.equal(pkgVersion);
+  });
+
+  it('astro dev', async () => {
+    const projectRootURL = new URL('./fixtures/astro-basic/', import.meta.url);
+
+    const proc = cli('dev', '--project-root', fileURLToPath(projectRootURL));
+
+    let stdout = '';
+
+    for await (const chunk of proc.stdout) {
+      stdout += chunk;
+
+      if (chunk.includes('Local:')) break;
+    }
+
+    proc.kill();
+
+    expect(stdout).to.include('Server started');
+  });
+
+  it('astro build', async () => {
+    const projectRootURL = new URL('./fixtures/astro-basic/', import.meta.url);
+
+    const proc = await cli('build', '--project-root', fileURLToPath(projectRootURL));
+
+    expect(proc.stdout).to.include('Done');
+  });
+});

packages/astro/test/config.test.js

@@ -1,5 +1,6 @@
 import { expect } from 'chai';
-import { devCLI, loadFixture } from './test-utils.js';
+import { cli, loadFixture } from './test-utils.js';
+import { fileURLToPath } from 'url';
 
 describe('config', () => {
   let hostnameFixture;
@@ -15,41 +16,40 @@ describe('config', () => {
     });
 
     it('can be specified via --hostname flag', async () => {
-      const cwd = './fixtures/config-hostname/';
-      const cwdURL = new URL(cwd, import.meta.url);
-      const args = ['--hostname', '127.0.0.1'];
-      const proc = devCLI(cwdURL, args);
+      const projectRootURL = new URL('./fixtures/astro-basic/', import.meta.url);
+      const proc = cli('dev', '--project-root', fileURLToPath(projectRootURL), '--hostname', '127.0.0.1');
 
-      proc.stdout.setEncoding('utf8');
+      let stdout = '';
 
       for await (const chunk of proc.stdout) {
-        if (/Local:/.test(chunk)) {
-          expect(chunk).to.include('127.0.0.1');
-          break;
-        }
+        stdout += chunk;
+
+        if (chunk.includes('Local:')) break;
       }
 
       proc.kill();
+
+      expect(stdout).to.include('127.0.0.1');
     });
   });
 
   describe('path', () => {
     it('can be passed via --config', async () => {
-      const cwd = './fixtures/config-path/';
-      const cwdURL = new URL(cwd, import.meta.url);
-      const configPath = new URL('./config/my-config.mjs', cwdURL).pathname;
-      const args = ['--config', configPath];
-      const proc = devCLI(cwdURL, args);
+      const projectRootURL = new URL('./fixtures/astro-basic/', import.meta.url);
+      const configFileURL = new URL('./fixtures/config-path/config/my-config.mjs', import.meta.url);
+      const proc = cli('dev', '--project-root', fileURLToPath(projectRootURL), '--config', configFileURL.pathname);
 
-      proc.stdout.setEncoding('utf8');
+      let stdout = '';
 
       for await (const chunk of proc.stdout) {
-        if (/Server started/.test(chunk)) {
-          break;
-        }
+        stdout += chunk;
+
+        if (chunk.includes('Local:')) break;
       }
 
       proc.kill();
+
+      expect(stdout).to.include('127.0.0.1');
     });
   });
 

packages/astro/test/fixtures/config-path/config/my-config.mjs

@@ -1,5 +1,6 @@
 export default {
-  renderers: [
-    '@astrojs/renderer-preact'
-  ]
+  devOptions: {
+    hostname: '127.0.0.1',
+    port: 8080,
+  },
 }

packages/astro/test/test-utils.js

@@ -99,11 +99,13 @@ function merge(a, b) {
   return c;
 }
 
-const cliURL = new URL('../astro.js', import.meta.url);
+const cliPath = fileURLToPath(new URL('../astro.js', import.meta.url));
 
-/** Start Dev server via CLI */
-export function devCLI(root, additionalArgs = []) {
-  const args = [cliURL.pathname, 'dev', '--project-root', root.pathname].concat(additionalArgs);
-  const proc = execa('node', args);
-  return proc;
+/** Returns a process running the Astro CLI. */
+export function cli(/** @type {string[]} */ ...args) {
+  const spawned = execa('node', [cliPath, ...args]);
+
+  spawned.stdout.setEncoding('utf8');
+
+  return spawned;
 }