Use when modifying, removing, or refactoring code that lacks test coverage. Emphasizes the danger of untested changes and the RGR workflow to add characterization tests before modifications.
Limited to specific tools
Additional assets for this skill
This skill is limited to using the following tools:
name: legacy-code-safety description: Use when modifying, removing, or refactoring code that lacks test coverage. Emphasizes the danger of untested changes and the RGR workflow to add characterization tests before modifications. allowed-tools:
"Changing untested code is like performing surgery blindfolded."
Modifying code without tests is one of the HIGHEST-RISK activities in software development.
When you change untested code:
In dynamically-typed languages (Python, JavaScript, Ruby, PHP), this risk is EXPONENTIALLY higher:
NEVER modify untested code without adding tests first.
If code lacks tests:
This is NOT standard TDD. This is characterization testing - capturing current behavior before changing it.
TDD: You know what the behavior SHOULD be, write test for ideal
Legacy RGR: You DON'T know all behaviors, capture what EXISTS
The "RED" step means "you're in the red zone" - working with dangerous untested code. Even though you expect the test to pass, you're at risk until you have that passing test as proof.
Before writing tests, understand what the code currently does:
// Legacy function - no tests
function calculateShipping(order) {
let cost = 9.99
if (order.total > 100) {
cost = 0
}
if (order.weight > 50) {
cost += 25
}
if (order.destination === 'HI' || order.destination === 'AK') {
cost += 15
}
return cost
}
Questions to answer:
Document what EXISTS, not what should exist:
describe('calculateShipping (characterization)', () => {
// Capture base case
test('standard shipping', () => {
const order = { total: 50, weight: 10, destination: 'CA' }
expect(calculateShipping(order)).toBe(9.99)
})
// Capture free shipping threshold
test('free shipping over $100', () => {
const order = { total: 150, weight: 10, destination: 'CA' }
expect(calculateShipping(order)).toBe(0)
})
// Capture weight surcharge
test('heavy package surcharge', () => {
const order = { total: 50, weight: 75, destination: 'CA' }
expect(calculateShipping(order)).toBe(34.99) // 9.99 + 25
})
// Capture regional surcharge
test('Hawaii surcharge', () => {
const order = { total: 50, weight: 10, destination: 'HI' }
expect(calculateShipping(order)).toBe(24.99) // 9.99 + 15
})
// Discovered edge case: free shipping + weight
test('free shipping with heavy package', () => {
const order = { total: 150, weight: 75, destination: 'CA' }
expect(calculateShipping(order)).toBe(25) // 0 + 25 (is this right?)
})
// Discovered edge case: all conditions
test('all surcharges with free shipping threshold', () => {
const order = { total: 150, weight: 75, destination: 'HI' }
expect(calculateShipping(order)).toBe(40) // 0 + 25 + 15
})
})
These tests MUST pass immediately:
npm test
If they don't pass:
With tests in place, you have a safety net:
// NOW we can safely refactor
function calculateShipping(order: Order): number {
const BASE_COST = 9.99
const FREE_SHIPPING_THRESHOLD = 100
const HEAVY_PACKAGE_WEIGHT = 50
const HEAVY_PACKAGE_SURCHARGE = 25
const REMOTE_DESTINATIONS = ['HI', 'AK']
const REMOTE_SURCHARGE = 15
let cost = order.total > FREE_SHIPPING_THRESHOLD ? 0 : BASE_COST
if (order.weight > HEAVY_PACKAGE_WEIGHT) {
cost += HEAVY_PACKAGE_SURCHARGE
}
if (REMOTE_DESTINATIONS.includes(order.destination)) {
cost += REMOTE_SURCHARGE
}
return cost
}
Run tests again:
npm test # All tests still pass
You've improved the code WITHOUT breaking existing behavior.
Use this skill when:
Even if the change seems "simple", if there are no tests, use this workflow.
Python, JavaScript, Ruby, PHP:
Extra caution required:
Go, Rust, TypeScript (strict mode), Java:
Type safety helps, but:
For large legacy systems, don't try to test everything at once:
Named after the strangler fig vine that grows around a tree:
Applied to code:
Phase 1: Wrap
// Legacy code (untested, scary)
function legacyCalculatePrice(item) {
// ... 500 lines of spaghetti ...
}
// New wrapper (tested)
function calculatePrice(item: Item): number {
// Add validation and tests here
validateItem(item)
// Call legacy for now
return legacyCalculatePrice(item)
}
Phase 2: Test the Boundary
describe('calculatePrice wrapper', () => {
test('validates input', () => {
expect(() => calculatePrice(null)).toThrow()
})
test('handles normal items', () => {
const item = { id: 1, price: 10 }
expect(calculatePrice(item)).toBe(10)
})
// Characterization tests for legacy behavior
test('matches legacy for discounts', () => {
const item = { id: 1, price: 10, discount: 0.2 }
expect(calculatePrice(item)).toBe(8)
})
})
Phase 3: Replace Internals
function calculatePrice(item: Item): number {
validateItem(item)
// New implementation (tested, clean)
const basePrice = item.price
const discount = item.discount ?? 0
return basePrice * (1 - discount)
// Legacy code removed!
}
Phase 4: Expand
Now that one function is safe, repeat for next function.
Before touching ANY untested code:
ONLY AFTER ALL CHECKS PASS:
Stop immediately if:
These are rationalizations for dangerous behavior.
Response: The initial change is fast. Finding and fixing the bugs you introduced takes much longer. Tests save time.
Response: If it's too complex to test, it's too complex to change safely. Simplify first, or use Strangler Fig pattern.
Response: Shipping a broken feature is slower than shipping a tested feature late. Technical debt compounds.
Response: Until now. The moment you change it, that safety is gone. Past stability doesn't predict future stability.
Response: Confidence is not a substitute for evidence. Even experts make mistakes.
How to know you've done this right:
Green flags:
Red flags:
Use these related skills:
Use coverage tools to identify untested code:
# JavaScript/TypeScript
npm test -- --coverage
# Python
pytest --cov=mypackage
# Ruby
bundle exec rspec --require spec_helper
# Go
go test -cover ./...
Target coverage:
Verify tests actually catch bugs:
# JavaScript
npm install -D stryker
# Python
pip install mutmut
Mutation testing changes your code slightly and verifies tests fail. If tests still pass after mutation, they're not effective.
For complex outputs (HTML, JSON, reports):
test('generates invoice', () => {
const invoice = generateInvoice(order)
expect(invoice).toMatchSnapshot()
})
First run captures current output. Future runs verify it hasn't changed.
Legacy payment processor, 2000 lines, no tests, needs to add new payment method.
Don't test everything - test what you'll touch:
// Will modify this
function processPayment(payment, method) {
// ... many lines ...
}
// Will NOT modify these
function logTransaction(transaction) { }
function sendEmail(user, content) { }
describe('processPayment (characterization)', () => {
test('processes credit card payment', () => {
const payment = { amount: 100, currency: 'USD' }
const method = { type: 'credit_card', number: '4111...' }
const result = processPayment(payment, method)
expect(result.success).toBe(true)
expect(result.transactionId).toBeDefined()
expect(result.amount).toBe(100)
})
// More tests for existing payment methods...
})
npm test
# ✅ All tests passing
function processPayment(payment, method) {
// Existing code preserved
if (method.type === 'credit_card') {
return processCreditCard(payment, method)
}
// NEW: Add cryptocurrency support
if (method.type === 'crypto') {
return processCrypto(payment, method)
}
throw new Error(`Unknown payment method: ${method.type}`)
}
test('processes cryptocurrency payment', () => {
const payment = { amount: 100, currency: 'USD' }
const method = { type: 'crypto', address: '0x123...' }
const result = processPayment(payment, method)
expect(result.success).toBe(true)
expect(result.transactionId).toBeDefined()
})
npm test
# ✅ All tests passing (old + new)
Legacy code is not an excuse for reckless changes. It's a reason for EXTRA caution.