Skip to content

Add e2e test coverage for --jq flag#308

Merged
jeremy merged 2 commits intomainfrom
bc-9679136022
Mar 16, 2026
Merged

Add e2e test coverage for --jq flag#308
jeremy merged 2 commits intomainfrom
bc-9679136022

Conversation

@jeremy
Copy link
Copy Markdown
Member

@jeremy jeremy commented Mar 15, 2026

Summary

  • Add 2 success-path tests in core.bats: --jq implies --json, scalar extraction works cleanly
  • Add 2 failure-path tests in errors.bats: invalid jq expression and --jq/--ids-only conflict both return structured usage errors

Locks in the behavior from #286.

Test plan

  • make test-e2e — 289/289 pass
  • make test — all unit tests pass

Summary by cubic

Add end-to-end tests for the --jq flag to lock in expected behavior and prevent regressions. Confirms that --jq implies --json, supports scalar extraction with exact-match output, and returns structured usage errors for invalid expressions and when combined with --ids-only; also tightens assertions and adds credentials setup in error tests.

Written for commit 6b9c289. Summary will update on new commits.

Lock in the behavior added in #286: --jq implies --json on success,
extracts scalars cleanly, rejects invalid expressions, and conflicts
with --ids-only.
@jeremy jeremy requested a review from a team as a code owner March 15, 2026 22:01
Copilot AI review requested due to automatic review settings March 15, 2026 22:01
@github-actions github-actions bot added tests Tests (unit and e2e) enhancement New feature or request labels Mar 15, 2026
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Adds end-to-end coverage for the new global --jq flag behavior (introduced in #286), ensuring both success and structured failure modes are locked in at the CLI boundary.

Changes:

  • Adds success-path e2e tests verifying --jq implies JSON output and supports scalar extraction.
  • Adds failure-path e2e tests verifying structured usage errors for invalid jq expressions and --jq conflicts (e.g., --ids-only).

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
e2e/core.bats Adds success-path --jq tests (implies JSON; scalar extraction output).
e2e/errors.bats Adds failure-path --jq tests for invalid expressions and flag conflicts returning usage errors.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 32c38033e4

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Copy link
Copy Markdown
Contributor

@cubic-dev-ai cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

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

5 issues found across 2 files

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="e2e/errors.bats">

<violation number="1" location="e2e/errors.bats:115">
P2: Missing `create_credentials` setup. Every other test in this file (including the neighboring "Global flag errors" tests for `--project`, `--account`, `--cache-dir`) calls `create_credentials` before `run basecamp`. Omitting it is inconsistent and risks flaky failures if credential-check ordering ever changes.</violation>

<violation number="2" location="e2e/errors.bats:120">
P2: Use `assert_json_value '.error'` instead of `assert_output_contains` for the error message. The suite convention (and every other structured-error test in this file) asserts the exact `.error` field value via `assert_json_value`, which is both stricter and more consistent.

(Based on your team's feedback about asserting the structured JSON error envelope using assert_json_value.) [FEEDBACK_USED]</violation>

<violation number="3" location="e2e/errors.bats:123">
P2: Missing `create_credentials` setup here as well, same as the sibling test above.</violation>

<violation number="4" location="e2e/errors.bats:128">
P2: Use `assert_json_value '.error'` instead of `assert_output_contains` here too, matching the suite convention.

(Based on your team's feedback about asserting the structured JSON error envelope using assert_json_value.) [FEEDBACK_USED]</violation>
</file>

<file name="e2e/core.bats">

<violation number="1" location="e2e/core.bats:83">
P2: `assert_output_contains "unauthenticated"` will match the unfiltered JSON envelope (which already contains that string at `.data.auth.status`), so this test passes even if `--jq` scalar extraction is completely broken. Use an exact-match assertion (e.g., `assert_output '"unauthenticated"'`) to verify the output is the extracted scalar, not the full JSON response.</violation>
</file>

Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.

- Exact-match scalar output instead of substring contains
- Use assert_json_value for conflict error (known exact string)
- Add create_credentials to error tests for convention consistency
@jeremy jeremy merged commit 59010c1 into main Mar 16, 2026
26 checks passed
@jeremy jeremy deleted the bc-9679136022 branch March 16, 2026 01:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request tests Tests (unit and e2e)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants