fix: Fix test infrastructure and add comprehensive test coverage [MBA-1487]#9
Open
devin-ai-integration[bot] wants to merge 6 commits intomainfrom
Open
fix: Fix test infrastructure and add comprehensive test coverage [MBA-1487]#9devin-ai-integration[bot] wants to merge 6 commits intomainfrom
devin-ai-integration[bot] wants to merge 6 commits intomainfrom
Conversation
Part A: Fix broken test infra - Update tsconfig.app.json with proper include/exclude for source files - Update tsconfig.spec.json with proper include for spec and source files - Remove broken zone.js imports from test-setup.ts and all existing spec files - Create app.component.spec.ts Part B: Fix Playwright E2E config - Add process.env.BASE_URL fallback to playwright.config.ts Part C: Add unit tests - Create TestComponent with Angular version display - Create TestDirectiveComponent with Angular version display - Create AppModule importing all standalone components - Add comprehensive unit tests for all new components Part D: Expand E2E tests - Add app-shell.spec.ts with heading, testid, console error, and visual regression tests Part E: Code coverage - Configure vitest coverage thresholds (80% statements) Co-Authored-By: unknown <>
Author
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
Co-Authored-By: unknown <>
Co-Authored-By: unknown <>
… E2E tests Co-Authored-By: unknown <>
Co-Authored-By: unknown <>
Co-Authored-By: unknown <>
Author
Test Results — MBA-1487Tested locally: unit tests, build, E2E, coverage, and frontend rendering. All Tests Passing
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes broken test tooling and adds comprehensive unit + E2E test coverage per MBA-1487.
Test infra fixes:
tsconfig.app.jsonto include allsrc/**/*.ts(excluding specs andtest-setup.ts) andtsconfig.spec.jsonto include both specs and source files; removed stale"types": ["jasmine"](project uses Vitest)zone.js/zone.js/testingimports fromtest-setup.tsand all 6 existing service spec files — the app uses zoneless Angular 21 (provideZonelessChangeDetection) andzone.jswas never installed as a dependencybeforeAllTestBed initialization from each spec (now handled centrally intest-setup.ts)New components & tests:
TestComponentandTestDirectiveComponentdisplayingVERSION.full, withdata-testidattributes for E2E targetingAppComponenttemplate alongside a<h2 data-testid="app-heading">Hello, angular-app</h2>headingAppModuleas an NgModule wrapper importing the standalone componentsAppComponent, andAppModule(191 total tests passing)E2E & Playwright config:
baseURLnow readsprocess.env.BASE_URLwith fallback tohttp://localhost:4200/e2e/app-shell.spec.tswith 4 tests: heading render, data-testid visibility, no JS console errors, visual regression (toHaveScreenshot)App-Shell-visual-regression-1-chromium-linux.png)Coverage:
@vitest/coverage-v8@4.0.18dev dependencyvitest.config.tsUpdates since last revision
<h1>to<h2 data-testid="app-heading">to avoid Playwright strict-mode violations (existing E2E tests usepage.locator('h1')which resolved to 2 elements with the original<h1>)src/test-setup.tsfromtsconfig.app.jsonper Devin Review feedback — it was being compiled duringng buildunnecessarilyapp-shell.spec.tsnow use[data-testid="app-heading"]instead of tag-based selectorsReview & Testing Checklist for Human
app.component.htmlnow renders<h2>Hello, angular-app</h2>,<app-test />, and<app-test-directive />above the router outlet on every page. This visibly changes the Conduit app UI. Confirm this is intentional and acceptable.TestComponent.ngOnInitandTestDirectiveComponentconstructor callconsole.info(...)on every page load. Verify this is desired for production builds.bun.lockresolved@angular/coreto21.2.4(was21.1.1), whilepackage.jsonstill pins21.1.1. Confirm this resolution change is acceptable or if it should be reverted.bun run test --runto verify all 191 unit tests passbun run test:e2eto verify all Playwright E2E tests pass (including visual regression baseline generation)bun run test:coverage -- --runto verify the 80% statement threshold is metSuggested manual test
bun run start→ openhttp://localhost:4200/Notes
AppModuleis not used at runtime (app bootstraps viabootstrapApplication); it exists solely to support the module compilation and render tests requested in the ticket.Link to Devin session: https://app.devin.ai/sessions/d81ab689be07495c9a65722550909cfd