Skip to content
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(registerMock): add registerMock functionality to register custom mocks per project #125

Merged
merged 10 commits into from Dec 31, 2019

Conversation

Pmyl
Copy link
Collaborator

@Pmyl Pmyl commented Dec 28, 2019

This PR adds the possibility to register a custom mock and it contains tests and documentation of every possible usage.
It also adds a new and better method of using extension strategies but I will create a new PR to add documentation for those.

This PR also fixes a usecase where an intersection with a typeof was breaking the build. Now an intersection with a typeof will ignore the typeof when deciding the generated type.

This closes #12

@Pmyl Pmyl marked this pull request as ready for review December 28, 2019 18:46
@Pmyl Pmyl requested a review from uittorio December 28, 2019 18:46
}

const factory: ts.FunctionExpression = node.arguments[0] as ts.FunctionExpression;
MockDefiner.instance.storeRegisterMockFor(TypescriptHelper.GetDeclarationFromNode((typeToMock as ts.TypeReferenceNode).typeName), factory);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if you want to avoid the casting to ts.TypeReferenceNode you can invert the if statement

You can also returning only once at the end with if else (I honestly don't mind either way).

Up to you :)

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

makes sense, I've inverted the if, removed the casting and removed duplication

@@ -9,7 +9,7 @@ export function GetIntersectionDescriptor(intersectionTypeNode: ts.IntersectionT
const nodes: ts.Node[] = GetTypes(intersectionTypeNode.types, scope);

const hasInvalidIntersections: boolean = nodes.some((node: ts.Node) => {
return TypescriptHelper.IsLiteralOrPrimitive(node);
return TypescriptHelper.IsLiteralOrPrimitive(node) || node.kind === ts.SyntaxKind.TypeQuery;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am trying to start using ts.isType instead of kind check... for example, ts.isTypeQuery(node)

Copy link
Collaborator Author

@Pmyl Pmyl Dec 28, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

interesting, I've changed it here, I think in busy places we should still use the check with kind to make it quicker

@@ -9,6 +10,10 @@ import { GetTypescriptType, IsTypescriptType } from '../tsLibs/typecriptLibs';
export function GetTypeReferenceDescriptor(node: ts.TypeReferenceNode, scope: Scope): ts.Expression {
const declaration: ts.Declaration = TypescriptHelper.GetDeclarationFromNode(node.typeName);

if (MockDefiner.instance.hasMockForDeclaration(declaration)) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Considering that now we check up front if we already have the mock shall we simplify the checks inside GetMockFactoryCall? I think there are unnecessary check in this scenario. If you look at this line

if (this._factoryCache.has(declaration)) {

we will check the declaration in the cache twice.

In the same way the last if statement that use GetMockFactory call could be simplified to not to the check because it will add a new declaration in the cache and create a new mock

Copy link
Collaborator Author

@Pmyl Pmyl Dec 30, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've changed few things:

  • now in mockFactoryCall.ts there are 2 different ways to get the call, one where you create a new one, one where you use an existing one, this makes sure a check done earlier is not repeated
  • in typeReference.ts we make the check and decide which function to use
  • in mockDefiner the ifs are removed

The only downside of this is that now there is no check anymore, a call to "GetMockFactoryCall" of an existing mock factory will create a call to the repository of a non existing factory. I would say that is ok, everything is under test so it shouldn't be possible to miss it.

return this._registerMockFactoryRegistrationsPerFile[sourceFile.fileName]
.map((reg: { key: ts.Declaration; factory: ts.Expression }) => {
const key: string = this._registerMockFactoryCache.get(reg.key);
// const key: string = this._factoryCache.get(reg.key);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you just forgot this line. If I understand the intention is good to have the registerFactoryCache separated so we can clean it when need it.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

removed, it was a leftover

const key: string = this.getDeclarationKeyMap(declaration);

this._registerMockFactoryCache.set(declaration, key);
// this._factoryCache.set(declaration, key);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you just forgot this line. If I understand the intention is good to have the registerFactoryCache separated so we can clean it when need it.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

removed, it was a leftover

Copy link
Member

@uittorio uittorio left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just few comments, let me know if they make sense

@Pmyl Pmyl changed the title add register mock feat(registerMock): add registerMock functionality to register custom mocks per project Dec 30, 2019
@uittorio uittorio merged commit 0feb05a into master Dec 31, 2019
uittorio pushed a commit that referenced this pull request Jan 4, 2020
… mocks per project (#125)

* add first working version of register mock, needs code cleaning and unit tests for extension parameters

* remove unwanted extra functionalities

* make sure register mock works fine when not using caching

* add new extension strategy example and write documentation for registerMock

* fix after rebase

* fix playground

* remove commented code

* simplify code

* use typescript methods to check node type

* remove duplicated check for mocked type
@uittorio uittorio deleted the registerMock branch January 5, 2020 14:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

add ability to define custom mocks for same type (example, Promises)
2 participants