feature: Add all resolvers unit tests #12 #55

regevbr · 2020-04-11T19:27:25Z

Changes:

add mutation testing using stryker (only runs on master as it takes very long time)
fixed commit linter
upload coverage reports from ci as artifacts so we can examine them if needed
updated all dependencies to latest versions
added support of specifying IOC bindings that can be used by tags or in multi inject
Added unit tests for db entities, to make sure the metadata is correct
added various loggings
Extracted the repository providers logic to an abstract class
Renamed flow.ts to dependencyChecker
Enhanced all CI revolvers to be more resilient, and added proper unit tests
Added a helper class to handle parsing NVM commands that can be used in CI configurations
Fixed target resolver to be able to handle more variations of node versions placeholders (like code names)
Various unit tests fixes to enhance coverage of mutation testings
Renamed lts.ts to nodeVersions as it can now handle more variations of node versions placeholders (like code names)
Changed unit tests framework to use extended-jest-mock which is typesafe
Added various test utilities
Moved tests that were in the unit tests directory to the integration tests directory, as they are not really unit tests
Use node 12 as the main node in CI as this is the node version being used in development
remove the usage of @injectable in interfaces as it is redundant - we only need injection in concrete classes
make the memoize decorator type safe

regevbr · 2020-04-11T19:40:23Z

@ynahmany this is a massive PR with 2 weeks works and I changed/added many features and touched many files.
This is definitely not how a PR should be, but I got crazy hahaha...

codecov · 2020-04-11T19:41:48Z

Codecov Report

Merging #55 into master will increase coverage by 8.26%.
The diff coverage is 96.80%.

@@            Coverage Diff             @@
##           master      #55      +/-   ##
==========================================
+ Coverage   89.64%   97.90%   +8.26%     
==========================================
  Files          63       75      +12     
  Lines         792     1387     +595     
  Branches       67      164      +97     
==========================================
+ Hits          710     1358     +648     
+ Misses         82       29      -53

Flag	Coverage Δ
#unittest	`97.90% <96.80%> (+8.26%)`	⬆️

Impacted Files	Coverage Δ
src/db/interfaces/IConnectionProvider.ts	`100.00% <ø> (ø)`
src/db/interfaces/IConnectionSettings.ts	`100.00% <ø> (ø)`
...solvers/cacheResolver/interfaces/ICacheResolver.ts	`100.00% <ø> (ø)`
src/resolvers/ciResolver/interfaces/ICIResolver.ts	`100.00% <ø> (ø)`
...lvers/ciResolver/interfaces/ISpecificCIResolver.ts	`100.00% <ø> (ø)`
.../resolvers/ciResolver/interfaces/ITargetMatcher.ts	`100.00% <ø> (ø)`
...ers/enginesResolver/interfaces/IEnginesResolver.ts	`100.00% <ø> (ø)`
...resolvers/testResolver/interfaces/ITestResolver.ts	`100.00% <ø> (ø)`
...c/utils/commandRunner/interfaces/ICommandRunner.ts	`100.00% <ø> (ø)`
src/utils/git/interfaces/IGitCheckout.ts	`100.00% <ø> (ø)`
... and 67 more

ynahmany

finish the review.
most of the tests - I reviewed, but my CR there was not that effective.

src/db/entities/dependency.ts

src/db/impl/AbstractRepositoryProvider.ts

ynahmany · 2020-04-15T16:28:36Z

src/db/index.ts

+
+export const EntitiesTags = {
+  dependencyVersion: Symbol.for(`DependencyVersion`),
+  dependency: Symbol.for(`Dependency`),


perhaps Dependency.Name will work? not sure though.

I will check, it might but I think it can cause a circular dependency

src/resolvers/cacheResolver/impl/cacheResolver.ts

src/resolvers/ciResolver/impl/resolvers/githubActions/githubActionsConfigParser.ts

src/resolvers/enginesResolver/impl/enginesResolver.ts

ynahmany · 2020-04-16T20:29:49Z

src/utils/memoize/memoize.ts

+    propertyKey: TKey,
+    descriptor: TypedPropertyDescriptor<GenericFunction<A>>
+  ): void => {
+    descriptor.value = memoizeFn(propertyKey, descriptor.value!, keyBuilder || defaultKeyBuilder);


note for myself to speak with you about it.

test/src/db/impl/dependencyVersionRepositoryProvider.spec.ts

ynahmany · 2020-04-19T11:57:33Z

test/src/resolvers/testResolver/index.spec.ts

@@ -1,4 +1,4 @@
-import { BindingTypes, testBindings } from '../../../common/bindingTester';
+import { BindingTypes, testBindings } from '../../../common/testers/bindingTester';
 import { ITestResolver, testResolverModulesBinder } from '../../../../src/resolvers/testResolver';
 import { TestResolver } from '../../../../src/resolvers/testResolver/impl/testResolver';



there is only import and no export here..

This is a test which invokes the testBindings method. No need to export anything.
Am I missing something?

so there is no invokation what so ever?

there is one just 1 line below

regevbr · 2020-04-19T14:11:57Z

@ynahmany thanks for the review!
I fixed what I deemed necessary and commented on the rest

regevbr self-assigned this Apr 11, 2020

regevbr added the chore Make the code better label Apr 11, 2020

regevbr requested a review from ynahmany April 11, 2020 19:38

regevbr added the pending code review pending code review label Apr 11, 2020

feat: Add all resolvers unit tests #12

ecab01d

regevbr force-pushed the #12 branch from 3c2b8d3 to ecab01d Compare April 11, 2020 21:28

regevbr added 5 commits April 12, 2020 11:51

feat: Add all resolvers unit tests #12

fa6cf66

feat: Add all resolvers unit tests #12

1e3bf92

feat: Add all resolvers unit tests #12

90e4d20

feat: Add all resolvers unit tests #12

d8cdd61

feat: Add all resolvers unit tests #12

0dfcd20

This was referenced Apr 14, 2020

chore: move to yarn 2 #56 #60

Closed

chore: consider replacing any usage of any with unknown #59 #62

Merged

bug: engines resolver ignores unbound semver ranges #50 #64

Merged

ynahmany reviewed Apr 19, 2020

View reviewed changes

regevbr added 4 commits April 19, 2020 16:17

feat: Add all resolvers unit tests #12

8ddd609

fix: engines resolver ignores unbound semver ranges #50

74c36de

fix: engines resolver ignores unbound semver ranges #50

8c67856

fix: engines resolver ignores unbound semver ranges #50

87ae3a8

ynahmany approved these changes Apr 19, 2020

View reviewed changes

regevbr merged commit 8c51e65 into master Apr 19, 2020

regevbr deleted the #12 branch April 27, 2020 13:33

regevbr removed the pending code review pending code review label May 8, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feature: Add all resolvers unit tests #12 #55

feature: Add all resolvers unit tests #12 #55

regevbr commented Apr 11, 2020 •

edited

Loading

regevbr commented Apr 11, 2020

codecov bot commented Apr 11, 2020 •

edited

Loading

ynahmany left a comment

ynahmany Apr 15, 2020

regevbr Apr 19, 2020

regevbr Apr 19, 2020

ynahmany Apr 16, 2020

ynahmany Apr 19, 2020

regevbr Apr 19, 2020 •

edited

Loading

ynahmany Apr 19, 2020

regevbr Apr 19, 2020

regevbr commented Apr 19, 2020

feature: Add all resolvers unit tests #12 #55

feature: Add all resolvers unit tests #12 #55

Conversation

regevbr commented Apr 11, 2020 • edited Loading

regevbr commented Apr 11, 2020

codecov bot commented Apr 11, 2020 • edited Loading

Codecov Report

ynahmany left a comment

Choose a reason for hiding this comment

ynahmany Apr 15, 2020

Choose a reason for hiding this comment

regevbr Apr 19, 2020

Choose a reason for hiding this comment

regevbr Apr 19, 2020

Choose a reason for hiding this comment

ynahmany Apr 16, 2020

Choose a reason for hiding this comment

ynahmany Apr 19, 2020

Choose a reason for hiding this comment

regevbr Apr 19, 2020 • edited Loading

Choose a reason for hiding this comment

ynahmany Apr 19, 2020

Choose a reason for hiding this comment

regevbr Apr 19, 2020

Choose a reason for hiding this comment

regevbr commented Apr 19, 2020

regevbr commented Apr 11, 2020 •

edited

Loading

codecov bot commented Apr 11, 2020 •

edited

Loading

regevbr Apr 19, 2020 •

edited

Loading