🧪 [testing improvement] Add tests for JulesClient constructor

[GreyC]

Added unit tests for the JulesClient constructor in packages/cli/src/client.ts. The tests verify that the apiKey is correctly set whether provided explicitly or via the JULES_API_KEY environment variable. To facilitate test execution in the provided environment using node --experimental-strip-types, import statements were updated to include explicit .ts extensions. Tests utilize the native node:test runner and node:assert library. Private property access in tests is handled by casting to any.

---

PR created automatically by Jules for task 7929658035887444011 started by @GreyC

[google-labs-jules]

👋 Jules, reporting for duty! I'm here to lend a hand with this pull request.

When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down.

I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job!

For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with @jules. You can find this option in the Pull Request section of your global Jules UI settings. You can always switch back!

New to Jules? Learn more at jules.google/docs.

---

For security, I will only act on instructions from the user who triggered this task.

[gemini-code-assist]

Summary of Changes

Hello, I'm Gemini Code Assist¹! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request significantly enhances the robustness of the JulesClient by introducing dedicated unit tests for its constructor. These tests ensure that the client reliably initializes with the correct API key, whether it's passed directly or sourced from environment variables, thereby preventing potential authentication issues. The changes also include minor adjustments to import statements to align with experimental Node.js module resolution.

Highlights

• New Unit Tests: Added comprehensive unit tests for the JulesClient constructor to ensure correct API key initialization.
• API Key Handling Verification: Verified that the JulesClient constructor correctly uses the API key whether it's provided explicitly or sourced from the JULES_API_KEY environment variable.
• Module Resolution Compatibility: Updated import paths in client.ts and the new test file to include .ts extensions, facilitating compatibility with node --experimental-strip-types.
• Native Node.js Testing: Utilized the native Node.js node:test runner and node:assert library for implementing the new tests.
• Private Property Access: Handled access to private apiKey property within tests by casting the client instance to any.

Changelog

• packages/cli/src/client.ts
  • Updated getApiKey import statement to include the .ts extension for module resolution.
• packages/cli/test/client.test.ts
  • Added a new test file (client.test.ts) dedicated to JulesClient constructor tests.
  • Implemented tests to verify that the apiKey is correctly set when provided as an argument.
  • Implemented tests to verify that the apiKey is correctly retrieved from the JULES_API_KEY environment variable when not explicitly provided.
  • Included necessary imports for JulesClient, node:assert, and node:test.

Activity

• No human activity has been recorded on this pull request yet.

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature	Command	Description
Code Review	/gemini review	Performs a code review for the current pull request in its current state.
Pull Request Summary	/gemini summary	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help	/gemini help	Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

[gemini-code-assist]

Code Review

This pull request adds unit tests for the JulesClient constructor, which is a great improvement. The tests verify that the API key is set correctly from both an explicit argument and from the environment variable. My feedback focuses on improving the test that handles the environment variable by using mocking. This will make the test more robust and isolated by avoiding modification of global state.

[GreyC]

/gemini review

[gemini-code-assist]

Code Review

This pull request adds unit tests for the JulesClient constructor, which is a great improvement. My review includes a suggestion to make the tests more robust by using mocking instead of manipulating environment variables.

I've also noticed the addition of a temp_verify directory, which appears to contain a copy of the code from packages/cli with modified import paths. Including duplicated code can lead to maintenance issues. It would be better to address the underlying need for explicit .ts extensions through build or test runner configuration (e.g., in tsconfig.json or the test command) rather than committing duplicated source files.

Additionally, I've left some comments on the files in temp_verify regarding error handling and the use of any types, as these are important for code quality. If these files are indeed temporary and will be removed, these comments can be disregarded for them but should be applied to the original source files in packages/cli.

[GreyC]

/gemini review

[gemini-code-assist]

Code Review

This pull request improves the API key handling by throwing an error instead of exiting the process when no key is found. It also adds unit tests for the JulesClient constructor. My review focuses on improving test coverage. I've suggested adding tests for sourcing the API key from a config file and for the error case when no key is found, which will make the tests more comprehensive.

[gemini-code-assist]

The current tests cover providing the API key directly and via an environment variable, which is great. However, to ensure full coverage of the JulesClient constructor's behavior, I recommend adding tests for two other important scenarios:

• When the API key is sourced from the configuration file.
• When no API key is available from any source, which should result in an error being thrown.

This will make the test suite more robust and prevent future regressions related to API key handling.

import { JulesClient } from '../src/client';
import assert from 'node:assert';
import { test } from 'node:test';
import * as fs from 'fs';

test('JulesClient constructor uses provided API key', () => {
  const apiKey = 'test-api-key';
  const client = new JulesClient(apiKey);
  assert.strictEqual((client as any).apiKey, apiKey);
});

test('JulesClient constructor uses API key from environment if not provided', (t) => {
  const originalEnvKey = process.env.JULES_API_KEY;
  process.env.JULES_API_KEY = 'env-api-key';
  t.after(() => {
    process.env.JULES_API_KEY = originalEnvKey;
  });

  const client = new JulesClient();
  assert.strictEqual((client as any).apiKey, 'env-api-key');
});

test('JulesClient constructor uses API key from config file', (t) => {
  const originalEnvKey = process.env.JULES_API_KEY;
  delete process.env.JULES_API_KEY;
  t.after(() => {
    process.env.JULES_API_KEY = originalEnvKey;
  });

  t.mock.method(fs, 'existsSync', () => true);
  t.mock.method(fs, 'readFileSync', () => JSON.stringify({ apiKey: 'config-api-key' }));

  const client = new JulesClient();
  assert.strictEqual((client as any).apiKey, 'config-api-key');
});

test('JulesClient constructor throws when no API key is found', (t) => {
  const originalEnvKey = process.env.JULES_API_KEY;
  delete process.env.JULES_API_KEY;
  t.after(() => {
    process.env.JULES_API_KEY = originalEnvKey;
  });

  t.mock.method(fs, 'existsSync', () => false);

  assert.throws(
    () => {
      new JulesClient();
    },
    {
      message: 'No API key found. Set JULES_API_KEY or run: jules_cli setup',
    }
  );
});

[GreyC]

@jules Please add the two missing test cases suggested here:

1. API key sourced from the config file (~/.config/jules/config.json) — mock the fs.readFileSync or the config reading logic to return a known key, and assert the constructor picks it up.
2. Error thrown when no API key is available from any source — ensure no env var, no config file, no explicit arg, and assert that constructing JulesClient throws an error.

Do not add any temp directories. Only modify packages/cli/test/client.test.ts.

[GreyC]

/gemini review

[gemini-code-assist]

Code Review

This pull request introduces valuable unit tests for the JulesClient constructor, covering various scenarios for API key resolution. The change in auth.ts to throw an error instead of exiting is a great improvement for testability and modularity. My review includes a few suggestions for the new test file to improve cross-platform compatibility and robustness, mainly by using path.join for constructing file paths and ensuring import paths include the .ts extension as mentioned in the PR description.

[gemini-code-assist]

Your PR description mentions using node --experimental-strip-types, which requires explicit file extensions in import paths. To ensure the tests can run in that environment, you should add the .ts extension to this import.

Suggested change

	import { JulesClient } from '../src/client';
	import { JulesClient } from '../src/client.ts';

[GreyC]

We cannot add .ts extensions here — this was already tried and reverted in commits 77197d0 and 18b1090 because tsc errors with TS5097 ('An import path can only end with a .ts extension when allowImportingTsExtensions is enabled'). The project compiles with tsc without allowImportingTsExtensions, so .ts extensions in imports are not valid. The test runner used is node:test via tsc-compiled output, not --experimental-strip-types directly.

[gemini-code-assist]

For cross-platform compatibility, it's better to construct file paths using path.join() instead of string concatenation. This will ensure the tests run correctly on different operating systems (e.g., Windows) by using the correct path separator. You'll need to add import path from 'node:path'; at the top of the file.

Suggested change

	const configPath = os.homedir() + '/.config/jules/config.json';
	const configPath = path.join(os.homedir(), '.config', 'jules', 'config.json');

[GreyC]

@jules Please fix both this and the comment on line 57 (id:2934907534) together in one commit:

1. Add import path from 'node:path'; to the imports in packages/cli/test/client.test.ts
2. Line 31: replace string concatenation with path.join(os.homedir(), '.config', 'jules', 'config.json')
3. Line 57: replace path.includes('.config/jules/config.json') with an exact match: path === path.join(os.homedir(), '.config', 'jules', 'config.json')

[google-labs-jules]

I've implemented the requested path handling improvements in packages/cli/test/client.test.ts. Specifically, I added the node:path import, used path.join() for the configuration file path, and updated the existsSync mock to use an exact path match for better robustness. All tests, including the new edge cases, are passing correctly.

[gemini-code-assist]

Using path.includes() for this check is brittle and might lead to false positives. It's more robust to perform an exact match against the full path. Constructing the path with path.join() also ensures cross-platform compatibility. Note that this requires importing the path module.

Suggested change

	if (path.includes('.config/jules/config.json')) return false;
	if (path === path.join(os.homedir(), '.config', 'jules', 'config.json')) return false;