-
Notifications
You must be signed in to change notification settings - Fork 490
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Resolve conflicts in files for test cases #74
Resolve conflicts in files for test cases #74
Conversation
Can you please check now. Please. |
src/app/owners/owner.service.spec.ts
Outdated
|
||
describe('#getOwners', () => { | ||
let expectedOwners: Owner[]; | ||
it("should return expected owners (called once)", () => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Did you have use the linter (ng lint
) ? I remember single quote should be used.
wq
Outdated
@@ -0,0 +1,15 @@ | |||
[33mea40ba4[m[33m ([m[1;36mHEAD -> [m[1;32mtestcases[m[33m, [m[1;31morigin/testcases[m[33m)[m HEAD@{0}: reset: moving to ea40ba4 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you remove this file ?
@@ -20,58 +20,55 @@ | |||
* @author Vitaliy Fedoriv |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you revert reformating please ?
import {Component, EventEmitter, OnInit, Output} from '@angular/core'; | ||
import {Specialty} from '../specialty'; | ||
import {SpecialtyService} from '../specialty.service'; | ||
import { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you revert reformating please ?
<label class="col-sm-1 control-label">Name</label> | ||
<div class="col-sm-6"> | ||
<input id="name" name="name" class="form-control" type="text" [(ngModel)]="specialty.name" #specialityName="ngModel"/> | ||
<input id="name" class="form-control" type="text" [(ngModel)]="speciality.name" name="specialityName" #specialityName="ngModel" /> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why does we have to 2 <input>
with the same id
?
@@ -32,6 +33,7 @@ import {Router} from '@angular/router'; | |||
}) | |||
export class OwnerAddComponent implements OnInit { | |||
|
|||
@ViewChild("ownerForm", { static: true }) ownerForm: NgForm; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why did you introduced the @ViewChild
decorator?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it was throwing error as ownerForm not found in component ts file
addOwner(owner: Owner): Observable<Owner> { | ||
return of(owner); | ||
} | ||
} | ||
|
||
describe('OwnerAddComponent', () => { | ||
describe("OwnerAddComponent", () => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Linting should be apply
karma.conf.js
Outdated
clearContext: false // leave Jasmine Spec Runner output visible in browser | ||
clearContext: false, // leave Jasmine Spec Runner output visible in browser | ||
jasmine:{ | ||
random: false |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why disabling random?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To run the test cases in order without random seed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Did you fall on errors due to random execution?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No i did not..i have removed now in latest commit.
Hi @arey tslint is not longer supporting, eslint does support as universal to typescript and js users. Can you please advice. |
Hi @arey tslint is not longer supporting, eslint does support as universal to typescript and js users. Can you please advice. |
It has test files ( spec files ) and certain changes had to be made to html file and ts files to pass test cases. All the needed files are committed in this PR.