-
Notifications
You must be signed in to change notification settings - Fork 251
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
feat(angular): update Karma config path in Angular preset #2548
feat(angular): update Karma config path in Angular preset #2548
Conversation
Trying to figure out why tests are failing in the It sounds like Possible changes affecting this:
This is the error reported:
SolutionAdding |
One last thing left to do: Decide whether the test in This is the test: import { expectMetrics } from '../../../helpers';
describe('After running stryker on angular project', () => {
it('should report 20% or 7% mutation score', async () => {
await expectMetrics({
mutationScore: 33.33,
killed: 2,
survived: 4,
noCoverage: 0,
compileErrors: 1,
})
// -------------------|---------|----------|-----------|------------|----------|---------|
// File | % score | # killed | # timeout | # survived | # no cov | # error |
// -------------------|---------|----------|-----------|------------|----------|---------|
// All files | 20.00 | 1 | 0 | 4 | 0 | 0 |
});
}); Test result:
The test description isn't reflecting the current assertions either. Please advice. |
c0c85d2
to
074a357
Compare
For me it looks good, tho - it seems we don't support angular 9.1>=? Do you have any idea if / when we can support it with the same setup like this one |
I also confirmed that Stryker 4 works for Angular versions 9.1 and 10.1. This is described in PR#41 for the Stryker Handbook and in the Stryker 4 announcement. I usually target the lowest supported version to make sure that there are no regressions. Maybe we could add another project and try to keep it updated to the most recent version of Angular (version 11.0's release date is November 11th 2020). |
7c5ef2a
to
59052c5
Compare
Yes, that is what we should do. I started doing this for mocha (we're supporting mocha@5 at the low end). It will make the e2e tests even slower (Angular needs a lot of dependencies), but there is no easy way around that without compromising the test. About the failing e2e test, I think it is a valid change since you removed the Could we maybe add the same tests for I'll have a proper look this week. |
I misread the 4.0 announcement post; I added the |
After I added back the TypeScript Checker, this error is reported:
Remember that this project uses TypeScript version 3.7.5. As seen in this TypeScript playground, Labeled tuple elements were added in TypeScript 4.0, so // range.ts
/**
* Represents a location in a file by range.
*/
type Range = [fromInclusive: number, toExclusive: number];
export default Range; This is the declarations output in // range.d.ts
/**
* Represents a location in a file by range.
*/
declare type Range = [fromInclusive: number, toExclusive: number];
export default Range;
//# sourceMappingURL=Range.d.ts.map Meaning TypeScript <4.0 is not supported for a bunch of the APIs. TypeScript 4.0 is supported by Angular 10.1. |
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.
Thanks for picking this up @LayZeeDK.
I personally don't think the performance test should have to be downgraded to 9.0. We should be performance testing Stryker itself, not Angular. Performance should change that much between angular releases (other than Angular specifics). Do you agree?
@@ -0,0 +1,13 @@ | |||
# Editor configuration, see https://editorconfig.org |
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.
The performance test doesn't have to be in this PR I think. We don't need that to be on the lower end of our Angular supported versions.
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.
I would still want it to follow current Angular CLI project structure, especially configurations such as karma.conf.js. I will upgrade it to Angular latest.
Unless you have something up your sleeve for fixing this bug right away, I will remove the TypeScript checker from the Angular 9.0 e2e project. Then I will add an Angular latest e2e project which will include the TypeScript checker. Finally, I will lock mutation report assertions so that we can get to merging this PR. |
Let me verify that this is a problem for Angular 9 projects outside of this monoreppo, using the NPM package directly. |
I figured out what the issue was. By default, Configuring However, the next issue arrises:
I actually know the issue here, since I did some investigation on this when migrating the angular project. The problem is that the tsconfig.app.json doesn't properly define which files are included in the project (for some reason). TypeScript will never be able to compile this normally, but since the angular team uses it's own compiler (Ivy?), it works for them. Changing this: {
"files": [
"src/main.ts",
"src/polyfills.ts"
],
"include": [
"src/**/*.d.ts"
]
} To this: {
"include": [
"src/main.ts",
"src/polyfills.ts",
"src/app/**/*",
"src/**/*.d.ts"
],
"exclude": [
"src/app/**/*.spec.ts"
]
} Works:
I think we should do the following here:
Do you agree @LayZeeDK and @kmdrGroch ? |
Where is this option added? "tsconfigFile": "tsconfig.app.json" It's an option for the TypeScript Checker in |
As noted in #2550, the TypeScript Checker works outside of this monorepo with Angular CLI's out-of-the-box configurations, even with Angular 9.0 and TypeScript 3.7 👍 |
Regarding |
9b299f0
to
59052c5
Compare
I agree. I removed TypeScript checker from For (1) and (3), I would like to use Angular 10 Strict mode ( |
Looks like we're ready to merge 👍 |
e2e
usingnpx -p @angular/[email protected] ng new (...)
to use default configurations and paths. Downgrading from Angular 9.1 to Angular 9.0 to make sure we support this version.perf
usingnpx -p @angular/[email protected] ng new (...)
to use default configurations and paths. Downgrading from Angular 9.1 to Angular 9.0 to make sure we support this version.strictTemplate: true
for now.e2e/test/angular-project/verify/verify.ts
because of Stryker configuration changes:perTest
coverage analysis since it's supported for Karma in Stryker 4.