Skip to content

docs: rewrite test documentation with structured developer guide#256

Open
Gokul-social wants to merge 2 commits intoopenml:mainfrom
Gokul-social:docs/write-test-documentation
Open

docs: rewrite test documentation with structured developer guide#256
Gokul-social wants to merge 2 commits intoopenml:mainfrom
Gokul-social:docs/write-test-documentation

Conversation

@Gokul-social
Copy link

This PR rewrites docs/contributing/tests.md to provide structured developer documentation for writing tests, addressing issue #169.

The previous version of tests.md consisted of informal engineering notes, benchmark measurements, and scattered implementation details. While useful internally, it was not structured as contributor-facing documentation.

This update converts the content into a clear testing guide aligned with the current repository structure and actual test implementation.

Addresses openml#169. Replaces informal benchmark notes with a structured
guide covering the four existing test categories (migration, integration,
direct database, direct function), performance tradeoffs, mocking
philosophy, and running instructions.
Copilot AI review requested due to automatic review settings February 25, 2026 17:56
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Feb 25, 2026

Walkthrough

This pull request updates documentation files with two main changes. The docs/contributing/tests.md file is revised to replace explanatory content about test fixtures and mocking with a comprehensive testing strategy guide covering test layers, infrastructure, test categorization (migration, integration, direct database, and direct function tests), performance tradeoffs, and practical instructions for running tests. Additionally, mkdocs.yml is modified to update the navigation structure by changing the "Intro" entry to "OpenML Server", adding a new "Changes" navigation item referencing migration documentation, and adjusting whitespace formatting around the Contributing section.

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly describes the main change: rewriting test documentation with a structured developer guide, which aligns with the primary file modification (docs/contributing/tests.md).
Description check ✅ Passed The description is directly related to the changeset, explaining the motivation and purpose of converting informal engineering notes into structured contributor-facing documentation for the tests.md file.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

Tip

Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs).
Share your feedback on Discord.


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.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

🧹 Nitpick comments (1)
docs/contributing/tests.md (1)

26-37: Document the known migration-test auth limitation explicitly.

Add a short caveat that some auth-failure paths (e.g., Code 103) cannot be validated through php_api parity checks when legacy responses are XML.

Suggested doc patch
 ### 1) Migration tests
 
 Migration tests compare Python API responses against the legacy PHP API for equivalent endpoints.
 These tests live under `tests/routers/openml/migration/`.
 
 Characteristics:
@@
 - Focus on compatibility guarantees during migration.
+
+Known limitation:
+
+- Some authentication failure paths (for example Code 103 on endpoints where legacy PHP returns XML)
+  cannot be reliably asserted via migration parity tests; cover those in Python unit/integration tests.

Based on learnings: Migration tests for endpoints relying on php_api cannot validate authentication failure paths (Code 103) when legacy PHP returns XML instead of JSON.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@docs/contributing/tests.md` around lines 26 - 37, Add a short caveat to the
"Migration tests" section that explicitly notes migration tests using the
php_api fixture cannot validate certain authentication-failure paths
(specifically "Code 103") when the legacy PHP API returns XML instead of JSON;
reference the "Migration tests" heading and mention php_api and Code 103 so
readers know the limitation and that parity checks won’t catch those
auth-failure responses when formats differ.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@docs/contributing/tests.md`:
- Around line 26-37: Add a short caveat to the "Migration tests" section that
explicitly notes migration tests using the php_api fixture cannot validate
certain authentication-failure paths (specifically "Code 103") when the legacy
PHP API returns XML instead of JSON; reference the "Migration tests" heading and
mention php_api and Code 103 so readers know the limitation and that parity
checks won’t catch those auth-failure responses when formats differ.

ℹ️ Review info

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 6a0fed5 and 4de5d38.

📒 Files selected for processing (2)
  • docs/contributing/tests.md
  • mkdocs.yml

Copy link

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

This PR rewrites the contributor-facing test documentation to be a structured guide describing the repository’s current test layers (migration, integration, database-level, and direct function tests), and updates the MkDocs navigation label to better match the project/site name.

Changes:

  • Replaced informal notes in docs/contributing/tests.md with a structured testing strategy guide (fixtures, test categories, tradeoffs, and how to run tests).
  • Updated mkdocs.yml nav label from “Intro” to “OpenML Server” for the index page.

Reviewed changes

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

File Description
mkdocs.yml Renames the top-level nav entry for index.md to “OpenML Server” (aligns with site naming).
docs/contributing/tests.md Full rewrite into a structured developer guide covering fixtures, test layers, tradeoffs, and common commands.

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

Copy link
Contributor

@sourcery-ai sourcery-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.

Hey - I've reviewed your changes and they look great!


Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants