Skip to content

fix: update tests to reflect migration template changes #11653

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.

Sign up for GitHub

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

Jump to bottom
Merged
gioboa merged 3 commits into from
Sep 17, 2025
Merged

fix: update tests to reflect migration template changes #11653

gioboa merged 3 commits into from
Sep 17, 2025

Conversation

@sgarner
Copy link
Collaborator

@sgarner sgarner commented Sep 16, 2025 •
edited by coderabbitai bot
Loading

#11490 changed the generated migration templates, but tests were not updated to match.

Summary by CodeRabbit

  • Documentation

    • Added JSDoc type hints and parameter annotations to migration docs and JavaScript templates.
    • Clarified examples to show both CommonJS and ES Module export styles.
    • Improved readability and editor type assistance; no runtime or API changes.

  • Tests

    • Relaxed one enum migration test to assert membership of expected values (order-insensitive); other tests unchanged.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Sep 16, 2025 •
edited
Loading

Walkthrough

Adds JSDoc QueryRunner typedefs and @param {QueryRunner} queryRunner annotations to JavaScript migration templates and docs; documents both CommonJS and ES Module export styles in examples. Also relaxes one test assertion to use membership check for enum values. No runtime or control-flow changes.

Changes

Cohort / File(s) Summary
Docs: Migrations examples
docs/docs/advanced-topics/1-migrations.md		 Added @typedef {import('typeorm').QueryRunner} QueryRunner and JSDoc @param {QueryRunner} queryRunner for up/down examples; show both module.exports = class … and export class … snippets.
JS migration templates (DB variants)
test/functional/commands/templates/generate/cockroachdb.ts, .../generate/mssql.ts, .../generate/mysql.ts, .../generate/oracle.ts, .../generate/postgres.ts, .../generate/sqlite.ts		 Inserted JSDoc @typedef {import('typeorm').QueryRunner} QueryRunner in header and added @param {QueryRunner} queryRunner JSDoc for up/down methods in the JavaScript template strings. No runtime/signature changes.
Result template
test/functional/commands/templates/result-templates-create.ts		 Added QueryRunner typedef and @param annotations inside the JS migration result template string. No runtime changes.
Tests: enum assertion relaxed
test/github-issues/6115/issue-6115.ts		 Changed up-migration assertion to expect(...).to.have.members([...]) and expanded expected operator values to include six members; down-migration assertion remains exact equality.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

Suggested labels

need review help, size-m

Suggested reviewers

  • alumni
  • gioboa
  • w3nl

Poem

In burrows of code I hop and write,
Typedefs tucked in JSDoc light.
Up and down the runners roam,
Exports paired to guide them home.
A carrot comment, clear and bright. 🥕

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The title "fix: update tests to reflect migration template changes" is a concise, single-sentence summary that accurately describes the primary change—updating tests to match modified migration templates—and matches the PR objectives and the provided file summaries.
Docstring Coverage ✅ Passed No functions found in the changes. Docstring coverage check skipped.
✨ Finishing touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch fix-migration-tests
📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between cc283da and 1d3a78f.

📒 Files selected for processing (1)
  • test/github-issues/6115/issue-6115.ts (1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings 
Learnt from: alumni
PR: typeorm/typeorm#11576
File: src/data-source/DataSource.ts:542-546
Timestamp: 2025-08-16T18:51:14.170Z
Learning: When improving type safety in TypeORM's query execution chain, adding generics to the QueryRunner.query method allows for proper type propagation from DataSource.query<T> through to QueryResult<T>, eliminating the need for type casts like 'as any' and 'as T'.
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (20)
  • GitHub Check: tests-linux (20) / sap
  • GitHub Check: tests-linux (20) / mongodb
  • GitHub Check: tests-linux (20) / cockroachdb
  • GitHub Check: tests-linux (20) / sqljs
  • GitHub Check: tests-linux (20) / oracle
  • GitHub Check: tests-linux (20) / better-sqlite3
  • GitHub Check: tests-linux (18) / sqljs
  • GitHub Check: tests-linux (18) / oracle
  • GitHub Check: tests-linux (18) / sap
  • GitHub Check: tests-linux (18) / postgres (17-3.5)
  • GitHub Check: tests-linux (18) / postgres (14-3.5)
  • GitHub Check: tests-linux (18) / mongodb
  • GitHub Check: tests-linux (18) / sqlite
  • GitHub Check: tests-linux (18) / mysql_mariadb
  • GitHub Check: tests-linux (18) / better-sqlite3
  • GitHub Check: tests-linux (18) / mssql
  • GitHub Check: tests-windows / sqljs
  • GitHub Check: tests-windows / better-sqlite3
  • GitHub Check: tests-windows / sqlite
  • GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (1)
test/github-issues/6115/issue-6115.ts (1)

63-70: LGTM: order-agnostic enum assertion with full-name labels is the right fix.

Using to.have.members([...]) with the expanded values matches the new template output and avoids brittle ordering assumptions.

Tip

👮 Agentic pre-merge checks are now available in preview!

Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.

  • Built-in checks – Quickly apply ready-made checks to enforce title conventions, require pull request descriptions that follow templates, validate linked issues for compliance, and more.
  • Custom agentic checks – Define your own rules using CodeRabbit’s advanced agentic capabilities to enforce organization-specific policies and workflows. For example, you can instruct CodeRabbit’s agent to verify that API documentation is updated whenever API schema files are modified in a PR. Note: Upto 5 custom checks are currently allowed during the preview period. Pricing for this feature will be announced in a few weeks.

Please see the documentation for more information.

Example:

reviews:
  pre_merge_checks:
    custom_checks:
      - name: "Undocumented Breaking Changes"
        mode: "warning"
        instructions: |
          Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).

Please share your feedback with us on this Discord post.

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@cloudflare-workers-and-pages
Copy link

cloudflare-workers-and-pages bot commented Sep 16, 2025

Deploying typeorm with  Cloudflare Pages  Cloudflare Pages

Latest commit: 1d3a78f
Status: ✅  Deploy successful!
Preview URL: https://39619432.typeorm.pages.dev
Branch Preview URL: https://fix-migration-tests.typeorm.pages.dev

View logs

@coveralls
Copy link

coveralls commented Sep 16, 2025

Coverage Status

coverage: 76.358%. first build
when pulling 1d3a78f on fix-migration-tests
into fa3cd43 on master.

OSA413
OSA413 approved these changes Sep 16, 2025
View reviewed changes
gioboa
gioboa approved these changes Sep 17, 2025
View reviewed changes
Copy link
Collaborator

@gioboa gioboa left a comment

Choose a reason for hiding this comment

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

Thanks @sgarner LGTM

@gioboa gioboa merged commit 3fac86b into master Sep 17, 2025
63 checks passed
@gioboa gioboa deleted the fix-migration-tests branch September 17, 2025 06:37
@sgarner sgarner mentioned this pull request Sep 19, 2025
Merged
7 tasks
ThbltLmr pushed a commit to ThbltLmr/typeorm that referenced this pull request Dec 2, 2025
@sgarner @ThbltLmr
fix: update tests to reflect migration template changes (typeorm#11653)
62de81c
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@naorpeled naorpeled naorpeled approved these changes

@OSA413 OSA413 OSA413 approved these changes

@gioboa gioboa gioboa approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@sgarner @coveralls @naorpeled @OSA413 @gioboa