Skip to content

docs: Add unit testing guide - #1606

Merged
jirispilka merged 7 commits into
masterfrom
docs/unit-testing-guide
Oct 19, 2026
Merged

jirispilka merged 7 commits into
masterfrom
docs/unit-testing-guide

Conversation

@jirispilka

@jirispilka jirispilka commented Oct 16, 2026 •

Copy link
Copy Markdown
Collaborator

This is a port of convention from apify-core as written by Pepa.
The number of unit-tests is exploding and this could help to have it under control 🤞🏻

Unit tests here have no written rules beyond naming and "follow existing patterns". Agent-written suites drift toward volume: tests that re-prove helpers with their own suites, it.each tables over inputs that take one code path, shared mutable mocks, and whole-sentence text pins, while error and fallback branches stay untested. Apify's platform repo solved this with a unit testing guide that agents must read before writing a test; this PR adapts it.

Follow-up: #1607 brings the existing unit and integration tests in line with the guide.

Drafted with Claude Code (AI-assisted).

🤖 Generated with Claude Code

Adapt Apify's internal unit testing guide to this repo: tests/AGENTS.md
with the read-before-writing gate and the test layer rule, and
tests/UNIT_TESTING_GUIDE.md with the scope, test double, assertion,
coverage and readability rules. The root test naming rule now names
describe blocks after behaviors.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@jirispilka jirispilka added the t-ai Issues owned by the AI team. label Oct 16, 2026
…MENT docs

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@jirispilka
jirispilka marked this pull request as ready for review October 16, 2026 07:44
@apify-service-account apify-service-account added the tested Temporary label used only programatically for some analytics. label Oct 16, 2026
@jirispilka jirispilka added the adhoc Ad-hoc unplanned task added during the sprint. label Oct 16, 2026

@RobertCrupa RobertCrupa left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Two of the new guidelines clash with existing tests:

  1. The new naming rule - many tests use describe('fn()')
  2. The "no hidden setup in beforeEach" rule - many test files use beforeEach

Is there be follow up Issue to fix these?

Comment thread tests/AGENTS.md Outdated
Comment thread tests/AGENTS.md Outdated
Comment thread tests/AGENTS.md Outdated
@jirispilka

Copy link
Copy Markdown
Collaborator Author

Two of the new guidelines clash with existing tests:

  1. The new naming rule - many tests use describe('fn()')
  2. The "no hidden setup in beforeEach" rule - many test files use beforeEach

Is there be follow up Issue to fix these?

Yes, follow up is the linked issue, the text there is a bit burried:

... for new and edited tests. Existing unit and integration tests predate it and break most of its rules. A sampled audit (15 unit files, about a third of the unit test code, plus every integration case file) found the following.

@MQ37 MQ37 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

  • examples in UNIT_TESTING_GUIDE.md come from the platform repo and don't exist here (makeUser, DEFAULT_ACCOUNTS, USER_KEYS, mockStep, api.slack.com), so an agent that greps for them finds nothing. the "One reviewed suite ..." stories are about 2 KB of the 10 KB every agent has to read in full on each unit test edit. I think one rule + one snippet with local names is enough, mockTask(overrides) is already a factory with overrides
  • the read-in-full gate is stated 5 times (tests/AGENTS.md:7 and :11, AGENTS.md:46, CONTRIBUTING.md:382, DEVELOPMENT.md:148). root AGENTS.md is always loaded so one sentence there + one callout in tests/AGENTS.md should do, CONTRIBUTING and DEVELOPMENT can just keep the link
  • tests/AGENTS.md:12 and :16 are the same rule, :18 points to the guide which points back here, and the layer list in :24-28 repeats the table in DEVELOPMENT.md. merge :12/:16, drop :18, link the table instead of repeating it

jirispilka and others added 4 commits October 19, 2026 10:17
Reverts 69e2af3 and 1076ba8. They softened every rule that caps test
count (wiring pair, same-path, positive/negative) and turned the checklist
into cover/keep only, against the guide's own thesis. Keeps the two
additions worth having: the mockApifyClient cast note and the merged
comment bullet.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@jirispilka

Copy link
Copy Markdown
Collaborator Author

Addressed in 1918754: anecdotes and platform-only names replaced with local ones (mockTask, MAX_TOOL_NAME_LENGTH), gate now stated once in root AGENTS.md plus the tests/AGENTS.md callout, duplicate lines and the layer list dropped in favor of the DEVELOPMENT.md table link.

@valekjo valekjo left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

🤞

@jirispilka
jirispilka merged commit 2b5d9ad into master Oct 19, 2026
15 checks passed
@jirispilka
jirispilka deleted the docs/unit-testing-guide branch October 19, 2026 09:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

adhoc Ad-hoc unplanned task added during the sprint. t-ai Issues owned by the AI team. tested Temporary label used only programatically for some analytics.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants