test: fix flaky test_unicorn_company by selecting from UNICORNS - FAANG_PLUS#298
Conversation
Fixes #297. `test_unicorn_company` was flaky because `get_company_tier()` checks FAANG_PLUS before UNICORNS, so a company in both sets resolves to 'faang_plus'. The old test took `next(iter(UNICORNS))`, which depends on PYTHONHASHSEED, and occasionally picked a FAANG+ unicorn — causing unrelated PRs (e.g. #295) to fail CI. Fix: select from `sorted(UNICORNS - FAANG_PLUS)` so the company is deterministic and is guaranteed to resolve to the 'unicorn' tier. Apply the same sort-for-determinism hardening to test_finance_sector_detected, test_defense_sector_detected, and test_company_can_overlap_tier_and_sector, so future tier-precedence refactors do not re-introduce flakiness. Verified across 10 PYTHONHASHSEED values (0..12345): 7/7 pass each time. Full suite: 718 passed.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request improves the determinism of tests in tests/test_enrichment.py by sorting sets before selecting elements, preventing issues caused by Python's hash seed randomization. It also introduces pytest.skip to explicitly skip tests when required configurations are missing. The review feedback suggests reorganizing the imports at the top of the file to comply with PEP 8 guidelines by separating standard library and third-party imports.
| import sys | ||
| import os | ||
| import math | ||
| import pytest | ||
| import requests | ||
| from datetime import datetime, timedelta, timezone, date | ||
| from unittest.mock import patch |
There was a problem hiding this comment.
According to PEP 8, imports should be grouped in the following order:
- Standard library imports
- Related third-party imports
- Local application/library specific imports
Each group should be separated by a blank line. Currently, standard library imports (sys, os, math, datetime, unittest.mock) and third-party imports (pytest, requests) are mixed together.
| import sys | |
| import os | |
| import math | |
| import pytest | |
| import requests | |
| from datetime import datetime, timedelta, timezone, date | |
| from unittest.mock import patch | |
| import math | |
| import os | |
| import sys | |
| from datetime import datetime, timedelta, timezone, date | |
| from unittest.mock import patch | |
| import pytest | |
| import requests |
References
- Imports should be grouped in the following order: standard library imports, third-party imports, and local imports, with a blank line separating each group. (link)
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
Pre-commit's end-of-file-fixer hook (--all-files) fails on every PR until the file on main has a trailing newline. The file is regenerated by automation that strips it; a follow-up should fix the generator (separate scope from this PR).
There was a problem hiding this comment.
Pull request overview
This PR hardens get_company_tier()-related tests against Python set iteration nondeterminism (hash-seed dependent ordering), eliminating CI flakiness when a selected company happens to exist in multiple classification sets.
Changes:
- Fix
test_unicorn_companyflakiness by selecting deterministically fromsorted(UNICORNS - FAANG_PLUS). - Make sector/overlap tests deterministic by sorting candidate sets before selecting an element.
- Replace silent “no-op” branches with explicit
pytest.skip(...)so empty categories are visible in test output.
|
Warning Review limit reached
More reviews will be available in 52 minutes and 18 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
Address gemini-code-assist review feedback (matches .gemini/styleguide.md). The pre-commit isort hook is scoped to scripts/*.py, so tests/ imports weren't auto-grouped.
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Address github-advanced-security alert. 'date' was imported but never used in this file (all in-file occurrences are inside docstrings or as part of identifiers like format_posted_date).
## Why
`docs/market-history.json` has been silently empty-on-categories for the
entire 90-day retention window. The aggregation step in
`save_market_history()` reads the wrong key.
`scripts/update_jobs.py` (pre-fix):
```python
for job in jobs:
for category in job.get('categories', []): # plural list, never populated post-enrichment
category_counts[category] += 1
```
But `enrich_jobs()` writes the singular form and never creates a
`categories` list:
```python
category = categorize_job(title, description)
job['category'] = category # {'id': 'software_engineering', 'name': ..., 'emoji': ...}
```
Reproduced against the live artifact:
```
$ python3 -c "import json; d=json.load(open('docs/market-history.json')); \
[print(s['date'], 'cats=', len(s['categories'])) for s in d['snapshots'][-5:]]"
2026-05-22 cats= 0
2026-05-23 cats= 0
2026-05-24 cats= 0
2026-05-25 cats= 0
2026-05-26 cats= 0
```
All 81 retained snapshots show `categories: {}`. The category breakdown
that feeds any "growing/declining categories" analytic has been dead.
## What
New module-level helper `iter_category_ids(job)`
(`scripts/update_jobs.py:2464`):
- Reads from `category.id` first (the enriched shape).
- Falls back to the legacy `categories` list **only** when the singular
path is absent or invalid — handles `None`, `{}`, `{'id': None}`,
`{'id': ''}`, non-list `categories` values, and non-string elements.
- Strips whitespace and skips empty values so malformed upstream data
can't poison the Counter.
`save_market_history()` switches to the new helper.
## Tests
`tests/test_save_market_history.py` adds four targeted regressions:
- `test_counts_categories_correctly_from_singular_category_field` — the
path that's broken on main today.
- `test_prefers_singular_category_field_over_legacy_categories_list` —
guards against double-counting if both shapes coexist.
- `test_falls_back_to_legacy_categories_list_when_category_missing` —
keeps old fixtures working.
-
`test_falls_back_to_legacy_categories_when_category_payload_is_invalid`
— partial-scrape recovery against four invalid singular shapes.
Existing snapshot-schema / retention / determinism assertions stay
green.
## Validation
- `pytest tests/test_save_market_history.py`: 25 passed.
- `pytest` (full suite): 722 passed.
- `pre-commit run --all-files`: clean.
- End-to-end smoke (run real `enrich_jobs()` → `save_market_history()`
on synthetic raw jobs, inspect resulting `docs/market-history.json`):
```
before fix: categories: {}
after fix: categories: {'software_engineering': 2, 'data_ml': 2}
```
Counts equal the enriched job count, as expected.
## Provenance
This is the same scoped fix from #274 (closed without merge despite
green CI), rebased cleanly onto current `main`. Original work by
@rvac-bucky — author attribution preserved in the commit. The
`docs/predictions.json` conflict from #274 doesn't re-occur because #298
already landed the trailing-newline normalization.
Closes #228
<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit
* **Refactor**
* Improved job category data normalization to support both legacy and
enriched data formats, enhancing system robustness and backward
compatibility.
* **Tests**
* Updated test suite to validate consistent category data handling and
standardized tier naming conventions.
<!-- review_stack_entry_start -->
[](https://app.coderabbit.ai/change-stack/ambicuity/New-Grad-Jobs/pull/299?utm_source=github_walkthrough&utm_medium=github&utm_campaign=change_stack)
<!-- review_stack_entry_end -->
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
---------
Co-authored-by: rvac-bucky <263012179+rvac-bucky@users.noreply.github.com>
](https://app.coderabbit.ai/change-stack/ambicuity/New-Grad-Jobs/pull/299?utm_source=github_walkthrough&utm_medium=github&utm_campaign=change_stack){kind=link}
Closes #297.
Summary
test_unicorn_companyflaked on PR feat(scraper): add support for weeks, months, and 'just posted' date formats #295's CI withassert 'faang_plus' == 'unicorn'. Root cause:get_company_tier()checksFAANG_PLUSbeforeUNICORNS, and the test usednext(iter(UNICORNS))— which depends onPYTHONHASHSEEDand occasionally lands on a company that is in both sets.sorted(UNICORNS - FAANG_PLUS)so the chosen company is guaranteed to resolve to'unicorn'.test_finance_sector_detected,test_defense_sector_detected, andtest_company_can_overlap_tier_and_sectorso a future tier-precedence change cannot reintroduce flakiness.pytest.skip(...)instead of a silentif ...:branch so that an empty config category is loudly visible.Test plan
pytest tests/test_enrichment.py -v— 93 passed.PYTHONHASHSEEDvalues (0, 1, 7, 13, 42, 99, 314, 1234, 9999, 12345): 7/7 of theTestGetCompanyTiercases pass every time.pytest): 718 passed.Risk
Test-only change. No production code touched.