From 7d2616dc6a596d55b6107cb71fdda10905d1f4a7 Mon Sep 17 00:00:00 2001 From: Aaron Louie Date: Mon, 27 Jan 2020 11:41:19 -0500 Subject: [PATCH] Fixes and adds unit tests --- src/app/app.component.spec.ts | 12 +- src/app/diagram/diagram.component.spec.ts | 125 +++++++++++------- src/app/diagram/diagram.component.ts | 23 ++-- src/app/file-list/file-list.component.spec.ts | 58 +++++++- src/app/file-list/file-list.component.ts | 1 - src/app/modeler/modeler.component.spec.ts | 38 +++++- src/app/modeler/modeler.component.ts | 10 +- .../workflow-spec-list.component.spec.ts | 44 +++++- .../workflow-spec-list.component.ts | 14 +- src/testing/mocks/diagram.mocks.ts | 72 ++++++++++ 10 files changed, 302 insertions(+), 95 deletions(-) diff --git a/src/app/app.component.spec.ts b/src/app/app.component.spec.ts index 0dc2e50..793d00f 100644 --- a/src/app/app.component.spec.ts +++ b/src/app/app.component.spec.ts @@ -1,6 +1,7 @@ -import { async, ComponentFixture, TestBed } from '@angular/core/testing'; +import {async, ComponentFixture, TestBed} from '@angular/core/testing'; +import {RouterTestingModule} from '@angular/router/testing'; -import { AppComponent } from './app.component'; +import {AppComponent} from './app.component'; describe('AppComponent', () => { let component: AppComponent; @@ -8,9 +9,12 @@ describe('AppComponent', () => { beforeEach(async(() => { TestBed.configureTestingModule({ - declarations: [ AppComponent ] + declarations: [AppComponent], + imports: [ + RouterTestingModule, + ] }) - .compileComponents(); + .compileComponents(); })); beforeEach(() => { diff --git a/src/app/diagram/diagram.component.spec.ts b/src/app/diagram/diagram.component.spec.ts index 8d54ec4..50b990f 100644 --- a/src/app/diagram/diagram.component.spec.ts +++ b/src/app/diagram/diagram.component.spec.ts @@ -1,22 +1,23 @@ import {HttpClientTestingModule, HttpTestingController} from '@angular/common/http/testing'; - import {DebugNode} from '@angular/core'; -import {async, ComponentFixture, getTestBed, TestBed} from '@angular/core/testing'; +import {async, ComponentFixture, TestBed} from '@angular/core/testing'; import {MatIconModule} from '@angular/material/icon'; import * as FileSaver from 'file-saver'; -import {ApiService, MockEnvironment} from 'sartography-workflow-lib'; -import {BPMN_DIAGRAM, BPMN_DIAGRAM_WITH_WARNINGS} from '../../testing/mocks/diagram.mocks'; +import {ApiService, FileType, MockEnvironment} from 'sartography-workflow-lib'; +import { + BPMN_DIAGRAM, + BPMN_DIAGRAM_WITH_WARNINGS, + DMN_DIAGRAM, + DMN_DIAGRAM_WITH_WARNINGS +} from '../../testing/mocks/diagram.mocks'; import {DiagramComponent} from './diagram.component'; describe('DiagramComponent', () => { - let httpMock: HttpTestingController; let fixture: ComponentFixture; let component: DebugNode['componentInstance']; - const injector: TestBed = getTestBed(); - beforeEach(async(() => { TestBed.configureTestingModule({ imports: [ @@ -32,7 +33,7 @@ describe('DiagramComponent', () => { fixture = TestBed.createComponent(DiagramComponent); component = fixture.debugElement.componentInstance; - httpMock = injector.get(HttpTestingController); + httpMock = TestBed.get(HttpTestingController); fixture.detectChanges(); })); @@ -41,80 +42,78 @@ describe('DiagramComponent', () => { fixture.destroy(); }); - it('should create and tear down', () => { expect(component).toBeTruthy(); }); - - it('should load and render successfully', (done) => { - - // given + it('should load and render BPMN successfully', (done) => { const diagramURL = 'some-url'; - component.importDone.subscribe(result => { - - // then expect(result).toEqual({ type: 'success', warnings: [] }); - done(); }); - // when component.loadUrl(diagramURL); - const request = httpMock.expectOne({url: diagramURL, method: 'GET'}); - request.flush(BPMN_DIAGRAM); }); - - it('should expose import warnings', (done) => { - - // given + it('should load and render DMN successfully', (done) => { const diagramURL = 'some-url'; - component.importDone.subscribe(result => { - // then - expect(result.type).toEqual('success'); - - expect(result.warnings.length).toEqual(1); - expect(result.warnings[0].message).toContain('unparsable content detected'); - + expect(result).toEqual({ + type: 'success', + warnings: [] + }); done(); }); - // when + component.loadUrl(diagramURL); + const request = httpMock.expectOne({url: diagramURL, method: 'GET'}); + request.flush(DMN_DIAGRAM); + }); + + it('should expose BPMN import warnings', (done) => { + const diagramURL = 'some-url'; + component.importDone.subscribe(result => { + expect(result.type).toEqual('success'); + expect(result.warnings.length).toEqual(1); + expect(result.warnings[0].message).toContain('unparsable content detected'); + done(); + }); component.loadUrl(diagramURL); const request = httpMock.expectOne({url: diagramURL, method: 'GET'}); - request.flush(BPMN_DIAGRAM_WITH_WARNINGS); }); - - it('should fail to load and render', (done) => { - - // given + it('should expose DMN import warnings', (done) => { const diagramURL = 'some-url'; - - // when + component.importDone.subscribe(result => { + expect(result.type).toEqual('success'); + expect(result.warnings.length).toEqual(1); + expect(result.warnings[0].message).toContain('unparsable content detected'); + done(); + }); component.loadUrl(diagramURL); - component.importDone.subscribe(result => { + const request = httpMock.expectOne({url: diagramURL, method: 'GET'}); + request.flush(DMN_DIAGRAM_WITH_WARNINGS); + }); - // then + it('should fail to load and render', (done) => { + const diagramURL = 'some-url'; + component.loadUrl(diagramURL); + component.importDone.subscribe(result => { expect(result.type).toEqual('error'); expect(result.error).toEqual('Http failure response for some-url: 404 FOO'); - done(); }); const request = httpMock.expectOne({url: diagramURL, method: 'GET'}); - request.flush('Not Found', { status: 404, statusText: 'FOO' @@ -134,19 +133,31 @@ describe('DiagramComponent', () => { }); it('should create a new diagram', () => { + const initializeModelerSpy = spyOn(component, 'initializeModeler').and.stub(); + const createDiagramSpy = spyOn(component.modeler, 'createDiagram').and.stub(); + component.openDiagram(); + expect(initializeModelerSpy).toHaveBeenCalledWith(undefined); + expect(createDiagramSpy).toHaveBeenCalled(); + }); + + it('should open an existing BPMN diagram from XML', () => { + const initializeBPMNModelerSpy = spyOn(component, 'initializeBPMNModeler').and.stub(); const importXMLSpy = spyOn(component.modeler, 'importXML').and.stub(); - component.createNewDiagram(); + component.openDiagram(BPMN_DIAGRAM, FileType.BPMN); + expect(initializeBPMNModelerSpy).toHaveBeenCalled(); expect(importXMLSpy).toHaveBeenCalled(); }); - it('should open an existing diagram from XML', () => { + it('should open an existing DMN diagram from XML', () => { + const initializeDMNModelerSpy = spyOn(component, 'initializeDMNModeler').and.stub(); const importXMLSpy = spyOn(component.modeler, 'importXML').and.stub(); - component.openDiagram(BPMN_DIAGRAM); + component.openDiagram(DMN_DIAGRAM, FileType.DMN); + expect(initializeDMNModelerSpy).toHaveBeenCalled(); expect(importXMLSpy).toHaveBeenCalled(); }); - it('should fail to open diagram', (done) => { - component.openDiagram('INVALID BPMN XML'); + it('should fail to open BPMN diagram', (done) => { + component.openDiagram('INVALID BPMN XML', FileType.BPMN); component.importDone.subscribe(result => { expect(result.type).toEqual('error'); expect(result.error.message).toContain('unparsable content INVALID BPMN XML detected'); @@ -154,14 +165,26 @@ describe('DiagramComponent', () => { }); }); + it('should fail to open DMN diagram', (done) => { + component.openDiagram('INVALID DMN XML', FileType.DMN); + component.importDone.subscribe(result => { + expect(result.type).toEqual('error'); + expect(result.error.message).toContain('unparsable content INVALID DMN XML detected'); + done(); + }); + }); + it('should edit diagram', () => { - const importXMLSpy = spyOn(component.modeler, 'importXML').and.stub(); + const initializeModelerSpy = spyOn(component, 'initializeModeler').and.stub(); + const onChangeSpy = spyOn(component, 'onChange').and.stub(); const createDiagramSpy = spyOn(component.modeler, 'createDiagram').and.stub(); - component.createNewDiagram(); + component.openDiagram(); + expect(initializeModelerSpy).toHaveBeenCalledWith(undefined); expect(createDiagramSpy).toHaveBeenCalled(); component.writeValue(BPMN_DIAGRAM); - expect(importXMLSpy).toHaveBeenCalled(); + expect(initializeModelerSpy).toHaveBeenCalledWith(undefined); + expect(onChangeSpy).toHaveBeenCalled(); }); it('should register onChange function', () => { diff --git a/src/app/diagram/diagram.component.ts b/src/app/diagram/diagram.component.ts index b6529b2..3e767c0 100644 --- a/src/app/diagram/diagram.component.ts +++ b/src/app/diagram/diagram.component.ts @@ -4,7 +4,7 @@ import {ControlValueAccessor} from '@angular/forms'; import BpmnModeler from 'bpmn-js/lib/Modeler'; import DmnModeler from 'dmn-js/lib/Modeler'; import * as fileSaver from 'file-saver'; -import {ApiService} from 'sartography-workflow-lib'; +import {ApiService, FileType} from 'sartography-workflow-lib'; import {BpmnWarning} from '../_interfaces/bpmn-warning'; import {ImportEvent} from '../_interfaces/import-event'; import {bpmnModelerConfig} from './bpmn-modeler-config'; @@ -19,6 +19,7 @@ export class DiagramComponent implements ControlValueAccessor, AfterViewInit { @ViewChild('containerRef', {static: true}) containerRef: ElementRef; @ViewChild('propertiesRef', {static: true}) propertiesRef: ElementRef; @Output() private importDone: EventEmitter = new EventEmitter(); + private diagramType: FileType = FileType.BPMN; private modeler: BpmnModeler | DmnModeler; private xml = ''; private disabled = false; @@ -33,12 +34,8 @@ export class DiagramComponent implements ControlValueAccessor, AfterViewInit { return this.xml; } - get properties(): any { - return this.modeler.get('propertiesPanel')._current; - } - ngAfterViewInit() { - this.initializeModeler('bpmn'); + this.initializeModeler(this.diagramType); this.openDiagram(this.xml); } @@ -48,10 +45,10 @@ export class DiagramComponent implements ControlValueAccessor, AfterViewInit { onTouched() { } - initializeModeler(diagramType) { + initializeModeler(diagramType: FileType) { this.clearElements(); - if (diagramType === 'dmn') { + if (diagramType === FileType.DMN) { this.initializeDMNModeler(); } else { this.initializeBPMNModeler(); @@ -85,11 +82,8 @@ export class DiagramComponent implements ControlValueAccessor, AfterViewInit { this.disabled = isDisabled; } - createNewDiagram() { - this.openDiagram(); - } - - openDiagram(xml?: string, diagramType?: string) { + openDiagram(xml?: string, diagramType?: FileType) { + this.diagramType = diagramType || FileType.BPMN; this.xml = xml; this.initializeModeler(diagramType); return this.zone.run(() => { @@ -137,7 +131,8 @@ export class DiagramComponent implements ControlValueAccessor, AfterViewInit { */ loadUrl(url: string) { this.api.getStringFromUrl(url).subscribe(xml => { - this.openDiagram(xml); + const diagramType = (xml.includes('dmn.xsd') ? FileType.DMN : FileType.BPMN); + this.openDiagram(xml, diagramType); }, error => this._handleErrors(error)); } diff --git a/src/app/file-list/file-list.component.spec.ts b/src/app/file-list/file-list.component.spec.ts index 3fe92de..b2865da 100644 --- a/src/app/file-list/file-list.component.spec.ts +++ b/src/app/file-list/file-list.component.spec.ts @@ -1,25 +1,73 @@ -import { async, ComponentFixture, TestBed } from '@angular/core/testing'; - -import { FileListComponent } from './file-list.component'; +import {HttpClientTestingModule, HttpTestingController} from '@angular/common/http/testing'; +import {async, ComponentFixture, TestBed} from '@angular/core/testing'; +import {MatIconModule} from '@angular/material/icon'; +import {MatListModule} from '@angular/material/list'; +import { + ApiService, + FileType, + MockEnvironment, + mockFileMeta0, + mockFileMetas, + mockWorkflowSpec0 +} from 'sartography-workflow-lib'; +import {FileListComponent} from './file-list.component'; describe('FileListComponent', () => { + let httpMock: HttpTestingController; let component: FileListComponent; let fixture: ComponentFixture; beforeEach(async(() => { TestBed.configureTestingModule({ - declarations: [ FileListComponent ] + imports: [ + HttpClientTestingModule, + MatIconModule, + MatListModule, + ], + declarations: [ + FileListComponent + ], + providers: [ + ApiService, + {provide: 'APP_ENVIRONMENT', useClass: MockEnvironment} + ] }) - .compileComponents(); + .compileComponents(); })); beforeEach(() => { fixture = TestBed.createComponent(FileListComponent); component = fixture.componentInstance; + httpMock = TestBed.get(HttpTestingController); + component.workflowSpec = mockWorkflowSpec0; fixture.detectChanges(); + + const fmsReq = httpMock.expectOne(`apiRoot/file?spec_id=${mockWorkflowSpec0.id}`); + expect(fmsReq.request.method).toEqual('GET'); + fmsReq.flush(mockFileMetas); + expect(component.fileMetas.length).toBeGreaterThan(0); + }); + + afterEach(() => { + httpMock.verify(); + fixture.destroy(); }); it('should create', () => { expect(component).toBeTruthy(); }); + + it('should get an icon code for each file type', () => { + Object.values(FileType).forEach(ft => expect(component.getIconCode(ft)).toBeTruthy()); + }); + + it('should delete a file', () => { + const loadFileMetasSpy = spyOn((component as any), 'loadFileMetas').and.stub(); + component.deleteFile(mockFileMeta0.id); + const fmsReq = httpMock.expectOne(`apiRoot/file/${mockFileMeta0.id}`); + expect(fmsReq.request.method).toEqual('DELETE'); + fmsReq.flush(null); + + expect(loadFileMetasSpy).toHaveBeenCalled(); + }); }); diff --git a/src/app/file-list/file-list.component.ts b/src/app/file-list/file-list.component.ts index f2a772b..693df4a 100644 --- a/src/app/file-list/file-list.component.ts +++ b/src/app/file-list/file-list.component.ts @@ -18,7 +18,6 @@ export class FileListComponent implements OnInit { } getIconCode(file_type: string) { - console.log('content_type', file_type); switch (file_type) { case FileType.BPMN: return 'account_tree'; diff --git a/src/app/modeler/modeler.component.spec.ts b/src/app/modeler/modeler.component.spec.ts index cbdce20..cdb8ade 100644 --- a/src/app/modeler/modeler.component.spec.ts +++ b/src/app/modeler/modeler.component.spec.ts @@ -15,18 +15,21 @@ import {BrowserDynamicTestingModule} from '@angular/platform-browser-dynamic/tes import {BrowserAnimationsModule, NoopAnimationsModule} from '@angular/platform-browser/animations'; import {of} from 'rxjs'; import { - ApiService, FileMeta, + ApiService, + FileMeta, + FileType, MockEnvironment, - mockFileMeta0, mockFileMetas, + mockFileMeta0, + mockFileMetas, mockWorkflowSpec0, mockWorkflowSpecs } from 'sartography-workflow-lib'; import {BPMN_DIAGRAM, BPMN_DIAGRAM_WITH_WARNINGS} from '../../testing/mocks/diagram.mocks'; import {BpmnWarning} from '../_interfaces/bpmn-warning'; import {FileMetaDialogData} from '../_interfaces/file-meta-dialog-data'; -import {ModelerComponent} from './modeler.component'; import {DiagramComponent} from '../diagram/diagram.component'; import {FileMetaDialogComponent} from '../file-meta-dialog/file-meta-dialog.component'; +import {ModelerComponent} from './modeler.component'; describe('ModelerComponent', () => { @@ -75,8 +78,30 @@ describe('ModelerComponent', () => { component = fixture.debugElement.componentInstance; component.diagramComponent = TestBed.createComponent(DiagramComponent).componentInstance; fixture.detectChanges(); + + const wfsReq = httpMock.expectOne('apiRoot/workflow-specification'); + expect(wfsReq.request.method).toEqual('GET'); + wfsReq.flush(mockWorkflowSpecs); + expect(component.workflowSpecs.length).toBeGreaterThan(0); + + mockWorkflowSpecs.forEach(wfs => { + const req = httpMock.expectOne(`apiRoot/file?spec_id=${wfs.id}`); + expect(req.request.method).toEqual('GET'); + req.flush(mockFileMetas); + + mockFileMetas.forEach((fm, i) => { + const fmReq = httpMock.expectOne(`apiRoot/file/${fm.id}/data`); + expect(fmReq.request.method).toEqual('GET'); + fmReq.flush(mockFileMetas[i].file); + }); + }); + })); + afterEach(() => { + httpMock.verify(); + fixture.destroy(); + }); it('should create', () => { expect(component).toBeTruthy(); @@ -154,8 +179,9 @@ describe('ModelerComponent', () => { }); const openDiagramSpy = spyOn(component.diagramComponent, 'openDiagram').and.stub(); const newFile = new File([BPMN_DIAGRAM], 'filename.xml', {type: 'text/xml'}); + component.diagramFileMeta = mockFileMeta0; component.readFile(newFile); - expect(openDiagramSpy).toHaveBeenCalledWith(BPMN_DIAGRAM); + expect(openDiagramSpy).toHaveBeenCalledWith(BPMN_DIAGRAM, FileType.BPMN); }); it('loads a diagram from File with error', () => { @@ -371,14 +397,18 @@ describe('ModelerComponent', () => { }); it('should get a file metadata display string', () => { + component.workflowSpecs = []; expect(component.getFileMetaDisplayString(mockFileMeta0)).toEqual('Loading...'); + component.workflowSpecs = mockWorkflowSpecs; const expectedString = 'all_things - all_things - Everything (one-fish.bpmn) - v1.0 (Jan 23, 2020)'; expect(component.getFileMetaDisplayString(mockFileMeta0)).toEqual(expectedString); }); it('should get file metadata tooltip text', () => { + component.workflowSpecs = []; expect(component.getFileMetaTooltipText(mockFileMeta0)).toEqual('Loading...'); + component.workflowSpecs = mockWorkflowSpecs; const expectedString = ` Workflow spec ID: all_things diff --git a/src/app/modeler/modeler.component.ts b/src/app/modeler/modeler.component.ts index 1b31954..ccc5cf6 100644 --- a/src/app/modeler/modeler.component.ts +++ b/src/app/modeler/modeler.component.ts @@ -41,14 +41,6 @@ export class ModelerComponent implements AfterViewInit { ngAfterViewInit(): void { this.diagramComponent.registerOnChange((newXmlValue: string) => { - - const props = this.diagramComponent.properties; - if (props) { - const entries = props.entries; - console.log('id', entries.id.oldValues.id); - console.log('name', entries.name.oldValues.name); - } - this.draftXml = newXmlValue; }); } @@ -143,7 +135,7 @@ export class ModelerComponent implements AfterViewInit { this.workflowSpec = undefined; this.diagramFileMeta = undefined; this.diagramFile = undefined; - this.diagramComponent.createNewDiagram(); + this.diagramComponent.openDiagram(); } private loadFilesFromDb() { diff --git a/src/app/workflow-spec-list/workflow-spec-list.component.spec.ts b/src/app/workflow-spec-list/workflow-spec-list.component.spec.ts index ad0cf05..7d3ceef 100644 --- a/src/app/workflow-spec-list/workflow-spec-list.component.spec.ts +++ b/src/app/workflow-spec-list/workflow-spec-list.component.spec.ts @@ -1,14 +1,34 @@ +import {HttpClientTestingModule, HttpTestingController} from '@angular/common/http/testing'; import { async, ComponentFixture, TestBed } from '@angular/core/testing'; +import {MatCardModule} from '@angular/material/card'; +import {MatIconModule} from '@angular/material/icon'; +import {MatListItem, MatListModule} from '@angular/material/list'; +import {ApiService, MockEnvironment, mockWorkflowSpec0, mockWorkflowSpecs} from 'sartography-workflow-lib'; +import {FileListComponent} from '../file-list/file-list.component'; import { WorkflowSpecListComponent } from './workflow-spec-list.component'; describe('WorkflowSpecListComponent', () => { + let httpMock: HttpTestingController; let component: WorkflowSpecListComponent; let fixture: ComponentFixture; beforeEach(async(() => { TestBed.configureTestingModule({ - declarations: [ WorkflowSpecListComponent ] + imports: [ + HttpClientTestingModule, + MatCardModule, + MatIconModule, + MatListModule, + ], + declarations: [ + FileListComponent, + WorkflowSpecListComponent + ], + providers: [ + ApiService, + {provide: 'APP_ENVIRONMENT', useClass: MockEnvironment} + ] }) .compileComponents(); })); @@ -16,10 +36,32 @@ describe('WorkflowSpecListComponent', () => { beforeEach(() => { fixture = TestBed.createComponent(WorkflowSpecListComponent); component = fixture.componentInstance; + httpMock = TestBed.get(HttpTestingController); fixture.detectChanges(); + + const sReq = httpMock.expectOne('apiRoot/workflow-specification'); + expect(sReq.request.method).toEqual('GET'); + sReq.flush(mockWorkflowSpecs); + + expect(component.workflowSpecs.length).toBeGreaterThan(0); + }); + + afterEach(() => { + httpMock.verify(); + fixture.destroy(); }); it('should create', () => { expect(component).toBeTruthy(); }); + + it('should delete a workflow spec', () => { + const loadWorkflowSpecsSpy = spyOn((component as any), 'loadWorkflowSpecs').and.stub(); + component.deleteWorkflowSpec(mockWorkflowSpec0.id); + const fmsReq = httpMock.expectOne(`apiRoot/workflow-specification/${mockWorkflowSpec0.id}`); + expect(fmsReq.request.method).toEqual('DELETE'); + fmsReq.flush(null); + + expect(loadWorkflowSpecsSpy).toHaveBeenCalled(); + }); }); diff --git a/src/app/workflow-spec-list/workflow-spec-list.component.ts b/src/app/workflow-spec-list/workflow-spec-list.component.ts index 7dffd10..d15acf8 100644 --- a/src/app/workflow-spec-list/workflow-spec-list.component.ts +++ b/src/app/workflow-spec-list/workflow-spec-list.component.ts @@ -10,20 +10,22 @@ export class WorkflowSpecListComponent implements OnInit { workflowSpecs: WorkflowSpec[] = []; constructor(private api: ApiService) { - this.api.getWorkflowSpecList().subscribe(wfs => { - console.log('wfs', wfs); - this.workflowSpecs = wfs; - }); + this.loadWorkflowSpecs(); } ngOnInit() { } newWorkflowSpec() { - } deleteWorkflowSpec(specId: string) { - this.api.deleteWorkflowSpecification(specId).subscribe(); + this.api.deleteWorkflowSpecification(specId).subscribe(() => this.loadWorkflowSpecs()); + } + + private loadWorkflowSpecs() { + this.api.getWorkflowSpecList().subscribe(wfs => { + this.workflowSpecs = wfs; + }); } } diff --git a/src/testing/mocks/diagram.mocks.ts b/src/testing/mocks/diagram.mocks.ts index c174c0d..bc9a947 100644 --- a/src/testing/mocks/diagram.mocks.ts +++ b/src/testing/mocks/diagram.mocks.ts @@ -29,3 +29,75 @@ export const BPMN_DIAGRAM_WITH_WARNINGS = ` `; + +export const DMN_DIAGRAM = ` + + + + + + + + + + + + + + + + 0 + + + "GREAT Dog! I love you." + + + + + 1 + + + "Oh, Ginger." + + + + + 2 + + + "Sheesh, you silly dog." + + + + + > 2 + + + "!@#$!@#$" + + + + + +`; + +export const DMN_DIAGRAM_WITH_WARNINGS = ` + + + + +`;