Get in Touch

Tag: Bulk Test Strategy

  • What a good Salesforce technical lead actually does in code review

    Most code review comments on a Salesforce PR are really about formatting. Bracket placement, an unused variable, a missing test class. None of that is wrong to flag, but it’s not the job. The job is catching the thing that passes every static check and still takes the org down at 50,000 records, three releases from now.

    Reading for what scales, not what compiles

    A trigger that queries inside a for loop will pass code coverage. It’ll pass a linter. It might even pass a demo with ten test records. It fails the first time someone runs a data load, and by then the person who wrote it has usually moved to a different project. Governor limits don’t show up in a diff. You have to know what the code will do at volume, which means picturing the bulk case before you read a single line of the actual implementation.

    I ask four things before I look at logic: what triggers this, how many records can hit it in one transaction, what happens if it runs twice, and whether it depends on a specific order of operations somewhere else in the org. Most defects I catch in review live in the gap between those questions and what the developer assumed while writing the happy path.

    The bulkification checklist nobody writes down

    SOQL and DML inside loops are the obvious ones, and most experienced Apex developers catch those themselves now. The subtler failures are recursive trigger calls, callouts fired per record instead of batched, and collection logic that looks bulk-safe but still does an expensive comparison somewhere in the middle. None of that fails a unit test with 200 records. It fails a Tuesday morning data load with 40,000.

    Test coverage tells you almost nothing

    Seventy-five percent is a compliance number, not a quality signal. A test class can hit every line and still assert nothing beyond “no exception was thrown.” When I review test code, I’m checking for negative cases: a null field, a duplicate external ID, a user without the right permission set, or a picklist value that no longer exists. If every test in the class uses the same clean data setup, the coverage number is decorative.

    The better question in review isn’t “does this have tests.” It’s “would this test fail if the logic were wrong.” Write the bug in your head, then check whether any assertion in the class would catch it. Half the time it wouldn’t, and that’s the actual finding, not the coverage percentage sitting at 82%.

    Where governance debt actually gets created

    Every org I’ve worked in that’s been customised for years has the same pattern behind it. Nobody made one bad decision. A hundred small, reasonable-looking decisions got approved individually, and the org ended up with several ways to do the same thing: a validation rule here, a flow there, an Apex trigger doing the same check somewhere else. Each PR looked fine on its own.

    Catching that in review means asking a question most reviewers skip: does this already exist somewhere else in the org? Before approving a new flow that enforces a business rule, I check whether that rule is already sitting in a validation rule, a different flow, or a trigger nobody’s touched in two years. It takes an extra five minutes. It’s the difference between one source of truth and three that quietly disagree with each other by the time someone debugs a production issue.

    Naming and structure aren’t nitpicks

    A trigger handler named after the object instead of the intent. A flow with no description. An Apex class mixing unrelated responsibilities because it was faster to bolt on than to split out. These get waved through constantly because none of them break anything today. What they break is the next person’s ability to safely change the org without reading every line first, from scratch, with no context. I’ll push back on structure even when the logic is correct, because the logic being correct once isn’t the bar. The bar is someone else understanding it without asking the original author, who by then might not even be on the project anymore.

    Security review isn’t a separate step

    Field-level security, sharing model behaviour, CRUD and FLS enforcement on SOQL: these tend to get treated as a checklist item for a security audit months later, if at all. They should be part of every review, not a phase that happens afterward. Is this Apex class running without sharing when it should respect it? Does this LWC expose a method that skips permission checks a profile would otherwise enforce? Catching that in review costs one comment. Catching it in production costs an incident report and a client conversation nobody wants to have.

    What I actually leave as comments

    Not “nice work,” and not a list of style preferences dressed up as requirements. I leave the specific scenario that breaks it: this fails when the account has no owner, this recursion loop triggers if the same record updates twice in one transaction, this assumes a single currency and the org has four. Specific, reproducible, tied to a real condition that exists in this org. A developer can act on that immediately. “Please refactor for clarity” gives them nothing to act on and just adds a review cycle for its own sake.

    Good review also says yes fast. If a PR does what it needs to do, handles bulk correctly, respects sharing, and the tests would actually catch a regression, approve it. Slowing down good work just to leave a comment teaches people to dread review instead of using it.

    The habit that makes the biggest difference

    This isn’t about being harsher on people’s code. Good review protects the org the code will run in six months, not the sandbox it happened to be written in. The developer who wrote it is usually thinking about today’s ticket. The reviewer’s job is thinking about the org at scale, under load, with the security model intact, and readable by whoever inherits it next year. That gap is the whole difference between a review that catches typos and one that actually protects the platform.

  • Apex testing that catches real bugs, not just coverage percentage

    Every Salesforce team hits 75% Apex code coverage eventually. Most of them still ship bugs the tests should have caught. Coverage tells you what ran. It doesn’t tell you what got checked.

    I’ve reviewed a lot of test classes over the years, across a fair few Salesforce orgs. The failure pattern repeats: a developer writes a test that exercises every line, asserts almost nothing, and calls it done. Deployment succeeds. Code review passes, because nobody reads test classes as closely as they read the actual logic. Then a bulk data load, or an edge case nobody thought about, finds the bug in production that the test never looked for.

    Coverage measures execution, not correctness

    The Salesforce test coverage number tells you which lines ran during a test. It says nothing about whether the test checked the right outcome, or any outcome at all. A test method can insert a record, call a method, and assert nothing, and Salesforce will count every line that executed anyway.

    I’ve seen test classes with a dozen methods and maybe three actual System.assert statements between them. They pass. They deploy. As regression protection they’re close to useless, and that’s the real cost. Six months later someone changes the trigger logic, nothing fails in CI, right up until a support ticket comes in.

    The three tells of a coverage-only test

    • It inserts one record, runs the method once, and checks that no exception was thrown. Nothing about the actual result.
    • It wraps everything in Test.startTest() and Test.stopTest() but never queries the database afterward to confirm the state changed the way it should.
    • It only tests the happy path. No bad input, no missing field, no record that’s already sitting in a status the code doesn’t expect.

    None of these are hard to spot in review. The problem is most reviewers are scanning for the coverage percentage on the deployment pipeline, not reading the test body line by line.

    What actually catches bugs before production does

    Test bulk, every time

    A trigger that works fine for one record and falls over at 150 is one of the most common incidents I’ve seen land in a support queue. Governor limits don’t show up in a single-record test. Write every trigger test against a batch, 200 records at minimum, because that’s the ceiling Salesforce enforces on a single DML operation, and it’s exactly where SOQL-in-a-loop and other bulkification mistakes surface.

    A contact-centre team I worked with had a case-assignment trigger that passed every test for two years. It queried a related object once per case, inside a loop. Fine for the sandbox tests, which always inserted one or two cases. Then a data migration loaded 3,000 cases in one batch and the transaction hit the SOQL query limit halfway through. The fix took an afternoon. Finding it took three days, because nobody had a bulk test that would have caught it in five seconds.

    Test the failure paths on purpose

    Negative testing gets skipped constantly, and it’s where the real bugs live. What happens when a required field is null? When a callout times out? When a record is locked by another process? When the user running the code doesn’t have edit access to the field your trigger is trying to update?

    Write a test for each of those, expect the exception, and assert on the actual error message or the DmlException type. If your Apex has a try/catch block, you need a test that proves the catch block does what you think it does. A surprising number of catch blocks quietly swallow errors that should have surfaced somewhere.

    Run tests as the people who’ll actually use the feature

    System.runAs() lets you execute a test in the context of a specific user, with their profile, their permission sets, their field-level security. Skip this and you’re testing as the org admin, who can see and edit everything. Deploy the same logic to a support agent on a restricted profile and it can fail in ways the test suite never flagged, usually a silent no-op where a field update just gets dropped rather than a clean exception.

    This matters more once sharing rules get involved. A “with sharing” class behaves differently depending on who’s running it. If your tests never run as a restricted user, you’re not testing the sharing model at all, just the Apex syntax.

    Build test data that looks like production, not a demo

    Real records have blank optional fields, statuses that skip stages, near-duplicate names, and relationships that don’t always resolve cleanly. A @TestSetup method that inserts three perfectly formed accounts with every field populated will never surface the null-pointer exception that shows up when a real account is missing its billing address.

    Build a small test data factory for your org’s core objects and reuse it everywhere. It’s less code overall than everyone hand-rolling their own test records, and when someone deliberately builds a messy edge case into the factory, every test in the org benefits from it at once.

    A short checklist for reviewing test classes

    When I review someone else’s Apex tests, I’m looking for a handful of things before I even check the coverage number:

    • Does every test method have at least one meaningful assertion, checking a value, not just confirming nothing threw?
    • Is there a bulk test with 150 to 200 records for anything that touches a trigger or a batch class?
    • Is there at least one test that deliberately fails, and asserts on how it fails?
    • Does anything sharing-sensitive get tested with System.runAs() as a restricted user?
    • Are HTTP callouts mocked with HttpCalloutMock, rather than skipped with scattered Test.isRunningTest() checks in production code?

    That last point deserves its own mention. I still see Test.isRunningTest() used inside actual business logic to skip a callout during tests. It works, technically. But it means the code path that runs in production is never the code path your test actually executes. A proper mock class takes more setup the first time, and it means the test is testing something real.

    Let coverage follow, don’t chase it

    Aim for tests that would catch a real bug if someone introduced one tomorrow: the bulk test, the negative test, the runAs test, the mocked callout. Do that consistently and the coverage percentage takes care of itself, usually well past 75% without anyone tracking the number directly.

    The org I trust is the one where a failing test means something actually broke. Not the one where every test class is green and the production incidents keep rolling in anyway.