Test Automation Best Practices and Code Quality for SDETs 2026 — The Complete Guide to Writing Maintainable, Scalable Test Code: Page Object Model Evolution From Monolithic Page Classes to Component-Based Architecture and When Each Pattern Still Makes Sense, SOLID Principles Applied to Test Automation With Concrete TypeScript and Python Examples Showing Single Responsibility in Test Classes and Dependency Inversion for Browser Abstractions, DRY Fixtures and Test Data Factories That Eliminate the Copy-Paste Anti-Pattern Without Creating Fragile Shared State, Naming Conventions That Survive Team Growth — How to Name Test Files, Test Cases, Page Objects, Fixtures, and Custom Assertions So a New Hire Can Navigate Your Test Suite in Their First Sprint, Code Review Standards for Test Automation That Go Beyond 'LGTM' — What Senior SDETs Actually Look For When Reviewing Test Pull Requests Including Assertion Quality, Selector Strategy, Wait Handling, and Test Independence Verification, the Most Common Test Automation Code Smells and How to Refactor Them — Sleep-Based Waits, Hard-Coded Test Data, God Page Objects, Assertion Roulette, Erratic Tests, Mystery Guest Tests, and Flaky Test Patterns That Erode Trust in Your Automation Suite, Architecture Patterns for Test Frameworks at Scale Including Layered Architecture, the Test Pyramid Enforced Through Code Review, and How to Design a Framework That 50 Engineers Can Contribute To Without Destroying Each Other's Tests, and How the SDET Interview Coach App Prepares You for the Framework Design and Code Quality Questions That Separate Mid-Level From Senior SDET Candidates in Technical Interviews
The definitive guide to test automation best practices and code quality standards for SDETs in 2026. You've felt it — that sinking feeling at 11 PM when your test suite that passed yesterday is now red with 37 failures, and you can't figure out whether the application broke or your tests broke. That's not a bad night. That's a design problem you can prevent. This guide covers the code quality principles, architectural patterns, naming conventions, and code review standards that transform test automation from a maintenance nightmare into a reliable engineering asset. Built from Mitchell Agoma's 20 years writing and reviewing test automation across HMRC, the Ministry of Defence, Nationwide Building Society, and Accenture — environments where test failures in production pipelines can mean regulatory findings, financial loss, or worse. You'll learn the Page Object Model evolution from monolithic page classes to component-based architecture, SOLID principles applied to test code with concrete examples in TypeScript and Python, DRY fixture patterns that eliminate copy-paste without creating fragile shared state, naming conventions that scale across teams, code review checklists that catch real problems before they reach main, and the most common test automation code smells with step-by-step refactoring recipes. Every section connects to the SDET Interview Coach iOS app — the tool that helps you practice articulating these principles in interview settings where framework design and code quality questions determine whether you're placed at mid, senior, or lead level.
Published 3 June 2026 • By Mitchell Agoma
Here is a moment every SDET knows: it's 11 PM, you're staring at a test run with 37 failures, and the worst part isn't the failures — it's that you can't tell whether the application broke or your tests broke. You open five test files and find the same login logic copied six different ways, a page object class that's 1,400 lines long, test data scattered across twelve JSON files with conflicting values, and a naming convention that made sense to one person in 2024 but makes sense to absolutely nobody in 2026. This is not a testing problem. This is a code quality problem — and it is the single biggest reason SDETs burn out, test suites get abandoned, and automation initiatives that started with executive enthusiasm end up as a folder nobody touches. Mitchell Agoma has spent 20 years writing, reviewing, and rescuing test automation across HMRC, the Ministry of Defence, Nationwide Building Society, and Accenture — environments where test failures in CI/CD pipelines can block regulatory releases, trigger audit findings, and cost real money. He has seen the same code quality anti-patterns appear in startups and global financial institutions, in JavaScript and Java, in Playwright and Selenium, on teams of two and teams of two hundred. The patterns that make test automation maintainable — or unmaintainable — are universal. This guide is the code quality playbook he wishes every SDET had before they wrote their first page object.
The difference between a mid-level SDET and a senior SDET is rarely about knowing more APIs. It's about the code quality instincts that prevent the 11 PM nightmare from happening in the first place. Senior SDETs write test code that other engineers can read, extend, and debug without a phone call. They design fixtures that don't create mysterious shared-state bugs. They name things so consistently that a new hire can navigate the test suite in their first sprint. And they review test pull requests with the same rigour that developers review production code — because in a modern CI/CD pipeline, test code is production code. If your tests are unmaintainable, your deployment pipeline is unreliable. If your deployment pipeline is unreliable, your velocity is fiction. This guide covers the principles and patterns that transform test automation from a source of anxiety into a source of confidence — and connects every section to the SDET Interview Coach iOS app, which includes code quality and framework design interview rounds calibrated to the seniority level you're targeting. Don't walk into a senior SDET interview without being able to discuss the code quality principles that separate architects from script-writers. Many SDET candidates struggle with articulating why they structure tests a certain way beyond "it's what we've always done." This guide gives you that vocabulary — and the reasoning behind it.
The Page Object Model in 2026 — From Monolithic Page Classes to Component-Based Architecture
The Page Object Model (POM) is the most widely used design pattern in test automation — and the most widely abused. Most SDETs learn POM as "one class per page, with methods for each user action and locators at the top." This works for a single-page application with five elements. It breaks catastrophically when your application has shared components (nav bars, modals, date pickers, notification toasts), pages with 50+ interactive elements, and multiple teams contributing to the same test suite. The monolithic page object anti-pattern is so pervasive that Mitchell has seen page classes exceeding 2,000 lines — classes where finding a single locator requires scrolling through a wall of selectors, and changing one footer link breaks 80 tests because the footer logic is duplicated across every page class that includes it. The fix is not abandoning POM — it's evolving it.
1. Component-Based Page Objects — Compose, Don't Inherit
The core insight: pages are composed of reusable components, and your page objects should reflect that. Instead of one monolithic CheckoutPage class containing locators for the header nav, the cart summary, the payment form, the address form, and the order confirmation modal, you compose it from component objects: HeaderNav, CartSummary, PaymentForm, AddressForm, ConfirmationModal. Each component owns its own locators, actions, and assertions. The page class becomes a thin coordinator: this.header = new HeaderNav(page); this.cart = new CartSummary(page);. When the header nav changes, you update one component class — not every page class that has a header. This is the Single Responsibility Principle applied to test architecture, and it's the single highest-leverage refactoring you can make to a bloated test suite. A senior SDET candidate should be able to explain not just what component-based POM is, but when the overhead of component extraction is justified versus when a slightly larger page class is acceptable — the trade-off nuance that interviewers at Lead level probe for.
2. The Screenflow Pattern — When User Journeys Span Multiple Pages
Page objects model individual pages. But user journeys — "add item to cart, apply discount code, select shipping, enter payment, confirm order" — span multiple pages. The Screenflow pattern (sometimes called Journey or Workflow pattern) models multi-step user flows as first-class abstractions. A CheckoutFlow class orchestrates CartPage, ShippingPage, PaymentPage, and ConfirmationPage into a single test-readable method: await checkout.completeOrder({ item: 'SKU-123', discount: 'SAVE10', shipping: 'express' }). This keeps your test cases at the level of business intent rather than page-level mechanics. Interviewers evaluating framework design answers want to hear that you understand the layering: tests call flows, flows call pages, pages call components. Each layer has a distinct responsibility, and none bleeds into the layer above it. The SDET Interview Coach app's framework design questions frequently probe this layering concept — can you articulate why a test should never directly interact with a page element locator?
3. Anti-Pattern: The God Page Object
The god page object is a single class that contains every locator, action, and assertion for an entire page — often exceeding 1,000 lines. Symptoms: you scroll for 30 seconds to find a locator; methods have names like clickSubmitButtonOnCheckoutPageAfterAddressEntry(); the class has 40+ import statements; merge conflicts happen on every PR because multiple people are editing the same file. The fix: extract shared components into their own classes, extract complex workflows into flow classes, and extract custom assertions into assertion helpers. A page class should be a thin wrapper — maybe 100-200 lines — that assembles components and exposes page-specific navigation. If your page class is longer than your test file, something is wrong. Mitchell has used this exact litmus test in code reviews across multiple organisations: if you can't see the entire page class on one screen, it's doing too much.
4. Anti-Pattern: Inheritance Hierarchies for Page Objects
Some frameworks build deep inheritance trees: BasePage → AuthenticatedPage → DashboardPage → AdminDashboardPage. This seems elegant at first — shared behaviour in the base class! — until you need to test a page that doesn't quite fit the hierarchy. Then you're overriding methods, adding conditional logic to base classes, and creating abstract methods that only some subclasses implement. The test automation community has largely moved away from inheritance-based POM toward composition-based POM for the same reason the broader software engineering community moved away from deep inheritance: composition is more flexible, more testable, and easier to reason about. Use base classes sparingly — for cross-cutting concerns like test lifecycle hooks, screenshot-on-failure configuration, and logger injection — not for page behaviour. A senior SDET should be able to articulate why they chose composition over inheritance, with reference to the fragility and tight coupling that deep hierarchies introduce. For more on design patterns in test frameworks, see our guide on test automation framework design interview questions.
SOLID Principles in Test Automation Code — They're Not Just for Production
Many SDETs treat SOLID principles as something developers worry about — not relevant to test code that "just needs to work." This mindset is why test suites rot. Test code that violates SOLID principles accumulates technical debt faster than production code because tests are inherently more coupled: they depend on application state, test data, environment configuration, and execution order. When any of those dependencies change, poorly structured tests break in ways that are expensive to diagnose. Applying SOLID principles to test automation code is not academic pedantry — it's the difference between a test suite that costs more to maintain than the bugs it catches and a test suite that genuinely protects your release pipeline. Let's walk through each principle with concrete test automation examples.
Single Responsibility Principle — One Reason to Change
In test automation, SRP means: a test should fail for exactly one reason. If a single test failure requires you to investigate the login flow, the test data setup, the API mocking, AND the UI assertion to understand what went wrong, that test violates SRP. Similarly, a page object method should do one thing: login(username, password) should log in — not also navigate to the login page, clear pre-filled fields, dismiss a cookie banner, and verify the redirect URL. Separate those concerns into separate methods or separate classes. The heuristic: when a test fails at 2 AM and the on-call engineer reads the failure message, can they identify the problem area in under 30 seconds? If not, your tests have unclear responsibilities. Mitchell has coached SDETs who were surprised to learn that this principle — not flaky locators — was the root cause of their test suite's high maintenance cost. Tests that fail for multiple reasons produce confusing failure messages that nobody trusts, which leads to the test being ignored or disabled — the death spiral of test automation.
Open/Closed Principle — Open for Extension, Closed for Modification
When you add a new test scenario, you should be able to extend existing abstractions — not modify them. The classic violation: a page object method with a boolean flag: submitForm(isExpressCheckout: boolean) that branches internally based on the flag. Every new checkout variant requires modifying the submitForm method, creating risk for existing tests. The fix: design page objects with extension points. Use the strategy pattern for variant behaviours: submitForm(strategy: CheckoutStrategy) where ExpressCheckoutStrategy and StandardCheckoutStrategy encapsulate their own logic. Use test data factories with builder patterns: userBuilder.withRole('admin').withSubscription('premium').build(). New scenarios add new builders or strategies — they don't modify existing ones. This principle is particularly testable in the SDET Interview Coach app's framework design mock interviews, where candidates are asked to refactor a flag-riddled page object into an open/closed design.
Liskov Substitution Principle — Subtypes Must Be Substitutable
In test automation, LSP violations appear when a subclass of a page object or component cannot be used wherever the parent is used. Example: you have a BaseModal with close() and confirm() methods. A ReadOnlyModal subclass overrides confirm() to throw an error because read-only modals can't be confirmed. Any test that iterates over a collection of modals calling confirm() will crash when it encounters a ReadOnlyModal. The fix: don't force inheritance relationships that don't make semantic sense. Use interfaces to define capabilities: Confirmable, Dismissable, Fillable. A ReadOnlyModal implements Dismissable but not Confirmable. This surfaces the contract violation at compile time rather than at test runtime. This level of design thinking — modelling test abstractions around capabilities rather than taxonomy — is what separates senior SDETs from mid-level SDETs in framework design interviews.
Interface Segregation Principle — No Client Should Depend on Methods It Doesn't Use
In test automation, ISP violations create tight coupling between tests and irrelevant page object methods. A ProductPage class with 40 methods — addToCart(), viewReviews(), checkStock(), compareProducts(), shareOnSocial(), etc. — forces every test that imports ProductPage to depend on methods it doesn't use. When the social sharing feature is removed, tests that only use addToCart() still need to be re-verified because they depend on the same class. The fix: split page objects by capability: ProductCartActions, ProductReviewActions, ProductComparisonActions. Tests import only the capabilities they need. This is particularly valuable in large test suites where different teams own different product areas — the cart team's tests shouldn't break when the reviews team changes their page object. The SDET Interview Coach app's framework design module includes ISP-focused questions that test candidates' ability to design loosely coupled test abstractions.
Dependency Inversion Principle — Depend on Abstractions, Not Concrete Implementations
In test automation, DIP means your tests should depend on abstractions — not on specific browser drivers, API clients, or database connections. The classic violation: tests that import Playwright's Page directly throughout the test body, coupling every test to the Playwright API. If you switch from Playwright to another framework, or upgrade Playwright to a version with breaking changes, every test file needs updating. The fix: introduce a Browser abstraction that wraps the framework-specific API. Your page objects depend on Browser, not on Playwright's Page. Your tests depend on page objects, not on the framework. The same principle applies to API clients (depending on an HttpClient interface rather than Axios directly), test data sources (depending on a TestDataProvider interface rather than a specific JSON file), and reporting (depending on a TestReporter interface rather than Allure directly). This is not abstract architecture for its own sake — Mitchell has personally migrated test suites between frameworks (Selenium to Playwright, REST Assured to SuperTest) and the abstractions that existed made the migration a project measured in weeks, while the tight coupling that remained made it a project measured in months. For deeper exploration of framework architecture, see our guide on test automation framework design interview questions.
DRY Fixtures and Test Data Factories — Eliminating Copy-Paste Without Creating Fragile Shared State
DRY (Don't Repeat Yourself) is the most misunderstood principle in test automation. Applied incorrectly, it creates the single most destructive anti-pattern in test suites: fragile shared test data. The scenario: you notice that six tests create the same user account with the same credentials. You extract a createDefaultUser() helper. The tests share the user. DRY achieved! Then test 3 modifies the user's email. Test 5 expects the original email. Test 5 fails. Test 6 runs after test 5 and inherits the modified state. Debugging this intermittent failure takes four hours. This is DRY applied without understanding test isolation — and it's the reason many SDETs develop an instinctive aversion to shared fixtures that is just as harmful as the original copy-paste. The solution is not abandoning DRY — it's applying DRY with isolation.
1. Immutable Test Data Factories — Create, Don't Share
A test data factory is a function that creates valid test data on demand with sensible defaults, allowing each test to override only the fields it cares about. In TypeScript: createUser({ role: 'admin' }) returns a complete user object with email, password, name, and role fields — all filled with unique values that won't collide with other tests. Every test gets its own data. No shared state. No ordering dependencies. The factory pattern is DRY because the generation logic is shared (you don't duplicate the user schema across tests) but the data instances are isolated (each test gets its own copy). Use unique identifiers per test run — a UUID or timestamp suffix — so even if cleanup fails, the next test run won't collide with stale data. Mitchell has implemented this pattern in fintech test suites at Nationwide Building Society where test data collisions in payment processing could mask real bugs — the factories eliminated an entire category of intermittent failures that had been plaguing the CI pipeline. The SDET Interview Coach iOS app includes test data strategy questions that probe whether candidates understand the difference between shared seeded data and isolated test data factories — a distinction that signals real framework design experience.
2. Fixture Scoping — The Lifetime of Test Dependencies
Most test frameworks provide fixture scoping: function-level (new fixture per test), class-level (shared across tests in a class), module-level (shared across tests in a file), and session-level (shared across the entire test run). The default should be function-level — the most isolated scope. Broader scopes should be used only when setup is genuinely expensive (database migrations, Docker container startup) and the shared resource is immutable during the test run. The heuristic: if you're using module-level fixtures for test data, you've probably created a shared-state bug that will surface at the worst possible moment. One pattern Mitchell recommends: use session-level fixtures for infrastructure (database connection pools, browser instances, Docker containers) and function-level fixtures for test data. Infrastructure is expensive to create and safe to share. Test data is cheap to create and dangerous to share. This distinction alone eliminates the majority of shared-state bugs in test suites. When SDET candidates can articulate fixture scoping decisions with this level of precision, interviewers take notice — it signals that you've actually operated a test suite at scale, not just written tests in a framework someone else designed.
3. The Builder Pattern for Complex Test Data
When test data has complex relationships — a user with multiple accounts, each account with transactions, each transaction with categories — a simple factory function becomes unwieldy. The builder pattern provides a fluent API for constructing complex test data: new UserBuilder().withAccount(accountBuilder.withBalance(1000).withTransactions(3)).build(). Builders enforce invariants (an account must have a positive balance for certain transaction types), provide sensible defaults for unspecified fields, and produce fully valid objects. The key design decision: builders should always produce valid objects by default. A UserBuilder().build() with no configuration should return a fully valid user — not a user with null fields that tests must then populate. This "valid-by-default" pattern means tests only specify what they care about, and the builder handles everything else. When interviewers ask "how do you manage test data in your framework?" builders demonstrate a level of design sophistication that goes beyond "I use JSON files."
4. When DRY Goes Too Far — The Danger of Over-Abstraction in Tests
There is a point where DRY becomes counterproductive in test code — and experienced SDETs learn to recognise it. The symptom: a test that calls five abstracted helper methods and the reader cannot determine what the test is testing without tracing through each helper. The test is DRY — no duplicated code — but it's unreadable. This is the "mystery guest" anti-pattern: the test's behaviour is hidden behind layers of abstraction that make the test intent opaque. The fix: keep the test's narrative visible. A test should read like documentation: "Given a user with an expired subscription, when they attempt to access premium content, then they see an upgrade prompt." The helpers should handle setup mechanics (creating the user, setting the subscription status) while the test body should remain at the level of business intent. If you have to click through three helper files to understand what a test does, your abstraction has gone too far. This is the nuance that separates thoughtful SDETs from cargo-cult DRY appliers — and it's exactly the kind of judgement that senior-level framework design questions probe. For more on test design patterns, see our guide on test case design techniques for SDET interviews.
Naming Conventions That Survive Team Growth — How to Name Tests, Files, and Helpers
Naming is the cheapest form of documentation and the most expensive form of technical debt. A test named test_login_2 costs nothing to write and costs hours to understand six months later. A test named login_with_expired_credentials_shows_appropriate_error_and_does_not_lock_account takes 30 seconds longer to write and saves every future reader 5 minutes. The naming conventions in your test suite are not cosmetic — they determine whether a new hire can contribute in their first week or their first month, whether an on-call engineer can diagnose a CI failure at 3 AM, and whether your test suite survives the original authors leaving the team. Mitchell has inherited test suites where the naming was so inconsistent that the first three months were spent renaming things just to understand what existed — time that could have been spent adding coverage. Here are the naming conventions that scale.
Test File Naming — Mirror the Application Structure
Test files should mirror the application structure they're testing. If your application has src/pages/checkout/, your test files should be at tests/checkout/. Don't organise tests by test type (tests/smoke/, tests/regression/) — organise them by application feature. The test type should be a tag or annotation on the test, not a directory structure. Why? Because when a checkout bug is reported, the developer should know exactly where to find the checkout tests without guessing whether they're in smoke, regression, or e2e. File naming convention: feature.spec.ts or feature.test.ts. Be consistent within the project. Avoid generic names like test1.ts, utils.ts, or helpers.ts — a file called helpers.ts becomes a dumping ground for anything that doesn't have a home. Name files for what they contain: login-helpers.ts, payment-assertions.ts, date-picker-component.ts. A consistent file naming convention means anyone can find the code they need without asking — and in a team of 50 engineers, not having to ask saves hundreds of cumulative hours.
Test Case Naming — The Should-When Pattern
The most readable test naming convention across the industry is the "should-when" pattern: should [expected behaviour] when [condition]. Examples: should display error message when login credentials are expired, should redirect to dashboard when user has valid session token, should show empty state when cart has no items. This pattern has three advantages: (1) it forces you to articulate the expected behaviour before writing the test, which catches vague test ideas early; (2) it reads naturally in test reports — a CI failure with the name "should display error message when login credentials are expired" immediately tells you what broke; (3) it's self-documenting — a test list becomes a behavioural specification of the application. Avoid pattern names like testLoginFlow or verifyCheckout — they describe the action but not the expectation. A test name should answer "what should happen?" not "what am I clicking?" The SDET Interview Coach app's coding challenge module reinforces this convention — candidates who name their test cases with the should-when pattern score higher on communication because the intent is immediately clear to the AI reviewer.
Page Object and Component Naming — Domain Language, Not Implementation Language
Page objects and components should be named using the language of the application domain, not the language of the implementation. LoginPage and CartSummary are domain names — they describe what the user sees. PageWithSidebarAndFooter is an implementation name — it describes how the page is structured. Domain names survive redesigns (the cart summary is still the cart summary even after a visual redesign); implementation names become misleading (the page no longer has a sidebar after the redesign, but the class is still called PageWithSidebarAndFooter). Similarly, component methods should use domain language: cart.addItem('SKU-123') not cart.clickAddButton(). The method should describe the user's intent, not the UI mechanics that fulfil it. This abstraction means your tests read like business documentation — and when the UI changes (a button becomes a swipe gesture on mobile), only the component implementation changes, not the test. This is the same principle behind BDD tools like Cucumber, but applied directly in your page object design rather than in a separate Gherkin layer that needs its own maintenance. For more on BDD patterns, see our guide on BDD and Cucumber interview questions.
Variables and Constants — Reveal Intent, Not Type
Variable names in test code should reveal intent, not type. const disabledSubmitButton = page.locator('button[disabled]') is better than const btn = page.locator('button[disabled]') because the name tells you what the element represents — not just that it's a button. const EXPIRED_USER_EMAIL = 'expired@test.com' is better than const TEST_EMAIL_1 = 'expired@test.com' because the name tells you why this email matters. Avoid Hungarian notation (btnSubmit, txtEmail) — modern IDEs show types, and the prefix adds noise without meaning. Avoid abbreviations that aren't universally understood within the team — addr might mean "address" to you but "adder" to someone else. The acid test: a developer who has never seen your test code before should be able to read a test and understand what it does without asking a single question. If they ask "what does this variable represent?" the name has failed. This standard is high, but it's the standard that separates professional test automation from script-writing — and it's a standard that Mitchell has enforced in code reviews across every organisation he's worked in. Candidates who can discuss naming conventions with this level of intentionality signal to interviewers that they've thought deeply about test maintainability — not just test creation.
The naming conventions you choose are less important than the consistency with which you apply them. A mediocre naming convention applied consistently across 5,000 tests is more navigable than an excellent naming convention applied inconsistently. Document your conventions in a one-page TESTING.md in your repository. Enforce them in code review. The SDET Interview Coach iOS app's behavioural interview module includes questions about how candidates enforce quality standards in their test suites — and naming convention enforcement is one of the most concrete examples of quality advocacy you can give.
Code Review for Test Automation — Beyond "LGTM"
In many organisations, test code reviews receive significantly less scrutiny than production code reviews. The logic is: "it's just tests — if they break, nobody dies." This logic is wrong in regulated industries (where test evidence is auditable), wrong in CI/CD pipelines (where test failures block deployments), and wrong in any team where test flakiness has eroded trust in automation. Test code reviews should be at least as rigorous as production code reviews — and they should evaluate criteria that are specific to test code. Mitchell has established test code review standards at HMRC and the Ministry of Defence where test automation evidence was subject to external audit — meaning a poorly reviewed test could result in a regulatory finding. Here is the code review checklist that catches real problems before they reach the main branch.
1. Assertion Quality — What Are We Actually Verifying?
The most common test code review finding: tests that interact with the application extensively but assert almost nothing. The test clicks through a five-step checkout flow and asserts that the confirmation page contains the word "success" — ignoring whether the correct items, quantities, prices, shipping method, and payment method are reflected. This is a weak assertion that gives false confidence. A code review should ask: does this test assert enough? Does it verify all the conditions that would indicate the feature is working correctly? Does it assert specific values, not just presence? Bonus: does it assert the absence of things that shouldn't be present (no error messages, no unexpected charges)? The counterbalance: assertion overkill. A test that asserts 50 things is brittle — any minor UI change breaks it, and the noise-to-signal ratio makes genuine failures hard to spot. The sweet spot: 3-7 meaningful assertions per test, each verifying a distinct business rule. More than 10 assertions in a single test case is a code smell that should trigger a discussion in review.
2. Selector Strategy — Will This Survive a Frontend Refactor?
Selectors are the most fragile part of any UI test — and the most common source of false-positive failures. A code review should evaluate selectors against a resilience hierarchy: (1) data-testid attributes — the most resilient, because they exist solely for testing and won't change during visual redesigns; (2) role-based selectors — resilient and accessible, because they describe what the element is (button, link, heading) rather than how it looks; (3) text content — moderately resilient, but breaks on copy changes; (4) CSS class selectors — fragile, because CSS classes change frequently during restyling; (5) XPath and nth-child selectors — the most fragile, breaking on any DOM structure change. A review should flag any XPath selector and any CSS selector that relies on implementation details. It should also flag selectors that are too broad — page.locator('button') when there are 15 buttons on the page — and too narrow — page.locator('div.container > div:nth-child(3) > span.text-primary') which breaks if anyone adds a wrapper div. The gold standard: every interactive element has a stable data-testid, and selectors use those IDs. If your application doesn't use test IDs, this is the code quality improvement that delivers the highest ROI for the lowest effort. The SDET Interview Coach app frequently tests selector strategy knowledge in its technical interview rounds — candidates who can articulate the resilience hierarchy demonstrate real-world framework experience.
3. Wait Handling — Sleep Is Never the Answer
The presence of await page.waitForTimeout(3000) or Thread.sleep(3000) in a test pull request should trigger an automatic request-for-changes. Hard-coded sleeps are the most expensive anti-pattern in test automation: a 3-second sleep on a test that runs 200 times per day wastes 10 minutes of CI time daily, 50 minutes weekly, and 43 hours annually — for a single test. Across a test suite of 500 tests, hard-coded sleeps can add hours to CI pipeline duration. The fix: explicit waits. page.waitForSelector('[data-testid="order-confirmation"]'), page.waitForResponse(resp => resp.url().includes('/api/order') && resp.status() === 200), expect(page.locator('.spinner')).not.toBeVisible(). Explicit waits are faster (they resolve as soon as the condition is met, not after a fixed delay), more reliable (they don't fail when the application is slower than usual), and self-documenting (the wait condition tells you what the test depends on). Mitchell's code review rule: any pull request containing a hard-coded sleep must include a comment justifying why an explicit wait is impossible — and those justifications are vanishingly rare. For a deeper dive into flakiness patterns, see our guide on test flakiness and stability interview questions.
4. Test Independence — Can These Tests Run in Any Order?
Test independence is the single most important property of a maintainable test suite. A test that depends on the side effects of a previous test is a time bomb — it will pass consistently in local development (where you always run tests in the same order) and fail intermittently in CI (where tests are parallelised and ordered differently). A code review should verify: does this test set up its own preconditions? Does it clean up after itself? Does it create unique data that won't collide with other tests? Does it assume any state from previous tests (a logged-in session, an item in the cart, a configured setting)? The most reliable litmus test: can you run this test in isolation — just this one test, not the whole file — and have it pass? If the answer is no, the test has a hidden dependency. In Playwright, this means using fixtures for test isolation rather than relying on test.describe.serial (which forces sequential execution and creates ordering dependencies). Candidates who can discuss test independence strategies — fixtures, unique data factories, idempotent setup — demonstrate that they've operated test suites in CI environments, not just local development. The SDET Interview Coach app's technical interview rounds probe test independence patterns because they're the most reliable signal of CI/CD experience.
The 7 Most Common Test Automation Code Smells — and How to Refactor Them
Code smells are surface indicators of deeper design problems. In test automation, code smells don't just make the code unpleasant to read — they directly cause the flakiness, slowness, and maintenance burden that lead teams to abandon automation. Here are the seven most common test automation code smells Mitchell has encountered across two decades of code reviews, with concrete refactoring recipes for each. Recognising these patterns is a prerequisite for senior-level SDET roles — and interviewers will frequently present a code sample containing one of these smells and ask you to critique it.
Sleep-Based Waits → Explicit Waits
Smell: await page.waitForTimeout(3000); scattered throughout tests. Why it's harmful: wastes CI time, still fails when the app is slower than 3 seconds, makes test duration unpredictable. Refactoring: replace every sleep with an explicit wait for a specific condition — element visibility, network idle, API response, or text content. Playwright's auto-waiting handles most cases; for custom conditions, use page.waitForFunction() or expect.poll(). Verification: run the test 20 times — it should pass every time without the sleep, at a fraction of the duration. If a test genuinely needs a sleep, it's a sign that the application has a race condition that should be fixed in the application, not worked around in the test.
Hard-Coded Test Data → Test Data Factories
Smell: literal values repeated across tests: email: 'test@example.com', password: 'Password123!', userId: 42. Why it's harmful: when the test data requirements change (passwords now require special characters), every test needs updating; tests collide when run in parallel because they use the same hard-coded identifiers; tests are brittle to database state changes. Refactoring: create test data factories that generate valid, unique data on demand. createUser({ role: 'admin' }) returns a complete user with a UUID-based email, a password that meets complexity requirements, and a unique ID. Verification: run the entire test suite in parallel — no data collisions. Run it against a fresh database — all tests pass. Run it against a database with existing data — tests create unique data that doesn't conflict.
Assertion Roulette → Targeted Assertions with Descriptive Messages
Smell: multiple assertions without descriptive messages, making it impossible to know which assertion failed from the test report: expect(page.url()).toContain('/dashboard'); expect(page.locator('h1')).toHaveText('Dashboard'); expect(page.locator('.user-name')).toBeVisible(); — failure output says "expected 'Dashboard' but got 'Error'" without telling you which assertion failed. Why it's harmful: debugging a failure requires reading the test code, not just the failure message — wasting time in CI investigations. Refactoring: add descriptive assertion messages: expect(page.url(), 'should redirect to dashboard after login').toContain('/dashboard'). Better: use soft assertions from expect.soft() to collect all failures in a single run rather than stopping at the first failure. Best: keep each test to one logical assertion (the one business rule it's verifying) and use helper methods with descriptive names for setup assertions. Verification: introduce a deliberate failure and check that the failure message tells you exactly what broke without reading the test code.
Erratic Test → Stable Test with Controlled Dependencies
Smell: a test that passes and fails intermittently without code changes — the classic "flaky test." Why it's harmful: erodes trust in the entire test suite; engineers learn to ignore CI failures, which means real failures get ignored too. Root causes (in order of frequency): (1) race conditions — the test checks a condition before the application has reached that state; (2) shared mutable state — another test modifies data this test depends on; (3) external dependency failures — a third-party API is down, the database is slow, the network is unreliable; (4) non-deterministic application behaviour — animations, random data, time-dependent logic; (5) actual intermittent bugs in the application. Refactoring: for race conditions, add explicit waits for the specific state the test needs. For shared state, isolate test data. For external dependencies, mock or stub them (Playwright's page.route() for API mocking). For animations, disable them in test environments. For time-dependent logic, use fake timers. Verification: run the test 100 times in CI — zero failures. Mitchell's threshold for "not flaky" is 100 consecutive passes. If a test can't achieve that, it shouldn't be in the CI pipeline. For comprehensive flakiness strategies, see our guide on test flakiness and stability interview questions.
Mystery Guest → Explicit Setup with Visible Test Data
Smell: the test calls helper functions that set up state invisibly — the reader cannot determine the test's preconditions from the test body. setupTestEnvironment(); loginAsUser(); navigateToCheckout(); — what user? What's in the cart? What payment methods are configured? Why it's harmful: when the test fails, you must trace through every helper to understand the starting state. When the test needs a new scenario, you don't know what state to modify. Refactoring: make preconditions explicit and visible in the test body. Use the Arrange-Act-Assert pattern with meaningful variable names: const user = await createUser({ subscription: 'expired' }); const page = await loginAs(user); await page.goto('/premium-content');. The reader knows exactly what's being tested. Helpers are for mechanics (creating the user, logging in), not for hiding intent. Verification: a colleague who has never seen the test can read it and describe the test scenario without asking questions.
Conditional Test Logic → Data-Driven Tests
Smell: if-else branches inside test bodies: if (user.role === 'admin') { await page.click('#admin-panel'); } else { await page.click('#user-panel'); }. Why it's harmful: conditional logic in tests means the test behaviour varies based on input — making it impossible to know from the test report which path was executed. It also means the test's assertions must handle multiple outcomes, weakening them. Refactoring: use data-driven testing (parameterised tests) instead of conditional logic. Each branch becomes its own test case with its own input data: test('admin user sees admin panel', ...) and test('regular user sees user panel', ...). The test framework runs each case independently with clear pass/fail reporting. Verification: every logical path through the original test has its own named test case that can pass or fail independently.
Copy-Paste Test Logic → Shared Helpers with Clear Contracts
Smell: the same 10-line setup sequence appears in 15 tests with minor variations. Why it's harmful: when the setup logic needs to change (a new required field in the form), every test must be updated individually — and inevitably one gets missed. Refactoring: extract the shared logic into a helper with a clear contract — what it requires, what it returns, what side effects it has. The helper should accept parameters for the parts that vary between tests and handle the invariant parts internally. Anti-pattern to avoid: the "helper that does everything" — a single setupCheckout() function with 15 optional parameters. This is just the copy-paste problem moved to a different file. Instead, create composable helpers: const user = await createAuthenticatedUser(); const cart = await populateCart(user, [item1, item2]); await applyDiscount(cart, 'SAVE10');. Each helper does exactly one thing. Verification: changing the setup logic in one place updates all tests that use it — and the tests still pass.
Recognising these code smells in a code review — and being able to articulate why they're harmful and how to refactor them — signals to interviewers that you've moved beyond writing tests into designing test systems. The SDET Interview Coach iOS app includes code review simulation exercises where you evaluate test code samples and identify code smells, calibrated to your target seniority level — a format that's increasingly common in senior SDET interview loops.
Architecture Patterns for Test Frameworks at Scale
The code quality principles covered so far operate at the level of individual tests and page objects. But when your test suite grows to thousands of tests across dozens of contributors, you need architectural patterns that enforce quality at scale — patterns that make it easy to do the right thing and hard to do the wrong thing. Mitchell has designed and evolved test framework architectures at HMRC (where test evidence was auditable by NAO), at the Ministry of Defence (where test environments were air-gapped), at Nationwide (where financial regulations required full traceability), and at Accenture (where frameworks were handed between client teams who had never seen them before). The patterns that worked across all four environments share common principles — and they're the patterns that senior and lead SDETs are expected to discuss in framework design interviews.
1. Layered Architecture — Tests, Flows, Pages, Components, Utilities
A well-architected test framework has distinct layers with clear dependency rules: layers can only depend on the layer directly below them. The layers, top to bottom: Test Layer — test cases that describe business scenarios in domain language, never directly interacting with page elements. Flow Layer (optional, for multi-page journeys) — orchestrates multiple pages into coherent user workflows. Page Layer — page objects that assemble components and expose page interactions. Component Layer — reusable UI component abstractions (nav bars, modals, tables, forms). Utility Layer — cross-cutting concerns: test data factories, API clients, authentication helpers, reporting, logging, configuration. The dependency rule: a test imports from flows; a flow imports from pages; a page imports from components and utilities; a component imports from utilities. Never the reverse. This layering means changes are contained — a UI redesign that changes button selectors affects only the component layer; test cases are untouched. This is the architecture Mitchell teaches SDET teams and the architecture that interviewers expect lead-level candidates to describe in detail. The SDET Interview Coach app's framework design interview round includes layered architecture questions that probe whether candidates understand dependency direction and layer responsibilities.
2. Shared Framework as a Versioned Library — Not a Copy-Pasted Template
When multiple teams contribute to the same test suite, the framework itself must be a shared, versioned library — not a template that each team copies and customises. This means: the framework lives in its own package (npm, PyPI, Maven) with semantic versioning; teams consume it as a dependency, not a fork; API changes follow deprecation cycles with migration guides; a CHANGELOG.md documents what changed and why. The shared library pattern prevents the "framework divergence" anti-pattern where Team A's copy has different helper signatures than Team B's copy, and tests that pass on Team A's branch fail on Team B's. It also creates a natural centre of excellence — a small group of framework maintainers who review contributions, enforce design standards, and keep the API clean. Mitchell has implemented this pattern at Accenture for multi-team banking clients where divergence between team frameworks was causing CI failures that nobody could diagnose because the failure was in the framework, not the test. For framework design interview questions, candidates who mention versioning, changelogs, and deprecation policies demonstrate experience operating frameworks at organisational scale — not just writing tests within a framework someone else maintains.
3. Configuration as Code — Environment Management Without Environment Confusion
Test frameworks that hard-code environment URLs, credentials, and feature flags are hostile to CI/CD. Every environment needs a different configuration, and configuration differences are a leading cause of "it works on my machine" failures. The pattern: configuration is code, stored in the repository, with environment-specific overrides. A config/default.ts defines base configuration (timeouts, retry counts, screenshot settings). Environment-specific files (config/staging.ts, config/production.ts) override only what differs. Secrets (API keys, passwords, tokens) are never in the repository — they're injected from CI/CD secret stores or environment variables. The framework validates configuration at startup — if a required environment variable is missing, it fails fast with a clear error message rather than failing mysteriously 45 minutes into the test run. The anti-pattern to avoid: configuration spread across the codebase — timeouts in individual test files, URLs hard-coded in page objects, credentials in test data files. Every configuration value should have a single source of truth. This is a pattern Mitchell has enforced in regulated environments where configuration management was audited — and it eliminates an entire category of environment-related test failures that waste SDET time. For more on environment management, see our guide on test environment management interview questions.
These architectural patterns are not theoretical — they are the patterns that Mitchell has implemented and evolved across regulated and commercial environments, and they're the patterns that SDET candidates at senior and lead levels are expected to discuss in framework design interviews. If you're preparing for a senior SDET interview, practice articulating not just what your framework architecture looks like, but why you chose that architecture — the trade-offs, the alternatives you rejected, and the problems you've encountered with other approaches. The SDET Interview Coach iOS app includes framework design interview rounds at every seniority level, with AI-powered mock interviews that probe your architectural reasoning — asking follow-up questions about trade-offs, scaling, and maintenance, exactly as a real interviewer would. If you're targeting a position where framework architecture is part of the job description, use Job Match to generate 50 bespoke questions from that specific JD — including the architecture and code quality questions that the hiring team will actually ask.
From Code Quality Principles to Interview Confidence — How SDET Interview Coach Prepares You
The principles in this guide — component-based POM, SOLID in test code, DRY fixtures with isolation, naming conventions that scale, code review standards, code smell recognition, and layered architecture — are exactly the principles that senior and lead SDET interviews test. Not through multiple-choice questions about definitions, but through scenario-based questions: "Here's a page object class — what's wrong with it?" "Our test suite takes 4 hours to run and fails 30% of the time — where do you start?" "Design a test framework for 10 engineering teams building a microservices platform." These questions require you to apply the principles, not just recite them — and that application fluency comes from practice.
The SDET Interview Coach iOS app is built for exactly this purpose. Its framework design and code quality topic areas contain questions calibrated to five seniority levels — from Junior (where you discuss basic POM and selector strategies) to Lead (where you design layered architectures for multi-team organisations). The AI mock interviewer asks follow-up questions, pushes back on incomplete answers, and scores your responses across technical accuracy, completeness, communication clarity, and code quality reasoning. The Job Match feature ingests a job description and generates 50 bespoke questions tuned to that specific role's expectations — so if the JD mentions "test framework architecture" or "code quality standards," you practice exactly those topics with questions crafted from the hiring team's own language. The spaced repetition system ensures the patterns, principles, and vocabulary in this guide move from your short-term memory into your long-term professional instincts — so when an interviewer asks about the Single Responsibility Principle in test automation, you don't fumble for an example; you have five ready.
Don't walk into a senior SDET interview without the code quality vocabulary and architectural reasoning that interviewers use to distinguish mid-level testers from senior engineers. The gap between "I write tests" and "I design test systems" is exactly what this guide — and the SDET Interview Coach app — is designed to close. If you're coming from a manual QA background, start with our guide on transitioning from manual QA to SDET. If you're building your testing methodology knowledge, see our guides on test case design techniques and test strategy and planning interview questions.
Ready to Transform Your Testing?
The AI Test Automation Playbook gives you everything you need: Playwright setup, Claude AI integration, MCP deep dive, 10+ ready-to-use prompts, CI/CD pipeline setup, and a 30-day implementation roadmap.
By Mitchell Agoma, Senior SDET & AI Testing Specialist with 8+ years of experience