just-jeb / jest-marbles

Helpers library for marbles testing with Jest
MIT License
113 stars 13 forks source link

After switching from jasmine-marbles to jest-marbles jest spyOn stopped to work #112

Closed felikf closed 3 years ago

felikf commented 5 years ago

After switching from jasmine-marbles to jest-marbles jest spyOn stopped to work:

https://stackoverflow.com/questions/55705040/marble-test-fails-with-jest-but-equivalent-test-succeeds-with-jasmine

With Jasmine is runs successful:

import { JasmineMarble } from './jasmine-marble';
import { cold } from 'jasmine-marbles';
import { switchMap } from 'rxjs/operators';
import { EMPTY, Observable } from 'rxjs';

class Service {
  foo(): Observable<any> {
    return EMPTY;
  }

  bar(a): Observable<any> {
    return EMPTY;
  }
}

describe('JasmineMarble', () => {
  it('should create an instance', () => {
    const service = new Service();

    spyOn(service, 'foo').and.returnValue(cold('a|', { a: 'A' }));
    spyOn(service, 'bar').and.returnValue(cold('a-b-c|', { a: 'A', b: 'B', c: 'C'}));

    const result$ = service.foo().pipe(switchMap(a => service.bar(a)));

    expect(result$).toBeObservable(cold('a-b-c|', { a: 'A', b: 'B', c: 'C'}));
    expect(service.bar).toHaveBeenCalledWith('A');
  });
});

With Jest it fails with this error trace:

expect(jest.fn()).toHaveBeenCalledWith(expected)

Expected mock function to have been called with:
  ["A"]
But it was not called.

The Jest code:

import { JestMarble } from './jest-marble';
import { cold } from 'jest-marbles';
import { switchMap } from 'rxjs/operators';
import { EMPTY, Observable } from 'rxjs';

class Service {
  foo(): Observable<any> {
    return EMPTY;
  }

  bar(a): Observable<any> {
    return EMPTY;
  }
}

describe('JestMarble', () => {
  it('should create an instance', () => {
    const service = new Service();

    jest.spyOn(service, 'foo').mockReturnValue(cold('a|', { a: 'A' }));
    jest.spyOn(service, 'bar').mockReturnValue(cold('a-b-c|', { a: 'A', b: 'B', c: 'C'}));

    const result$ = service.foo().pipe(switchMap(a => service.bar(a)));

    expect(result$).toBeObservable(cold('a-b-c|', { a: 'A', b: 'B', c: 'C'}));
    expect(service.bar).toHaveBeenCalledWith('A');
  });
});

You can find an example repository here: https://github.com/stijnvn/marbles The Jasmine example can be run with ng test jasmine-marbles. The Jest one with ng test jest-marbles.

just-jeb commented 5 years ago

If you run it in a clear Jest repository (no Jasmine stuff installed) it still reproduces?

felikf commented 5 years ago

I can reproduce it with following package.json, there is no direct dependency on Jasmine:

{
  "name": "project1",
  "version": "0.0.0",
  "license": "MIT",
  "scripts": {
    "ng": "ng",
    "start": "ng serve --aot  --base-href /applications/project1_dev/",
    "build": "node --max_old_space_size=8192 node_modules/@angular/cli/bin/ng build --base-href /applications/project1/ --configuration=deva",
    "build-mock": "node --max_old_space_size=8192 node_modules/@angular/cli/bin/ng build --aot --base-href /applications/project1mock/ --configuration=devaMock",
    "build-deva": "node --max_old_space_size=8192 node_modules/@angular/cli/bin/ng build --base-href /applications/project1_deva/ --configuration=deva",
    "build-devb": "node --max_old_space_size=8192 node_modules/@angular/cli/bin/ng build --base-href /applications/project1_devb/ --configuration=devb",
    "build-devc": "node --max_old_space_size=8192 node_modules/@angular/cli/bin/ng build --base-href /applications/project1_devc/ --configuration=devc",
    "build-prod": "node --max_old_space_size=8192 node_modules/@angular/cli/bin/ng build --base-href /applications/project1/ --prod",
    "test": "jest",
    "test-ci": "jest",
    "lint": "ng lint",
    "lint-ci": "ng lint --format tslint-teamcity-reporter",
    "bundle-report": "webpack-bundle-analyzer dist/project1/stats.json",
    "e2e": "ng e2e"
  },
  "private": true,
  "dependencies": {
    "@angular/animations": "=7.2.15",
    "@angular/common": "=7.2.15",
    "@angular/compiler": "=7.2.15",
    "@angular/core": "=7.2.15",
    "@angular/forms": "=7.2.15",
    "@angular/http": "=7.2.15",
    "@angular/platform-browser": "=7.2.15",
    "@angular/platform-browser-dynamic": "=7.2.15",
    "@angular/router": "=7.2.15",
    "@angular/material": "=7.3.7",
    "@angular/cdk": "=7.3.7",
    "@angular/flex-layout": "=7.0.0-beta.24",
    "@angular/material-moment-adapter": "=7.3.7",
    "@ngrx/effects": "=7.4.0",
    "@ngrx/router-store": "=7.4.0",
    "@ngrx/store": "=7.4.0",
    "@ngrx/store-devtools": "=7.4.0",
    "@ngx-translate/core": "=11.0.1",
    "@ngx-translate/http-loader": "=4.0.0",
    "core-js": "=2.5.7",
    "error-stack-parser": "=2.0.2",
    "hammerjs": "=2.0.8",
    "lodash": "=4.17.11",
    "moment": "=2.24.0",
    "ngx-take-until-destroy": "=5.4.0",
    "node-sass": "=4.12.0",
    "rxjs": "=6.5.2",
    "ts-md5": "=1.2.4",
    "web-animations-js": "=2.3.1",
    "zone.js": "=0.8.29"
  },
  "devDependencies": {
    "@angular-builders/jest": "=7.4.2",
    "@angular-devkit/build-angular": "=0.13.9",
    "@angular/cli": "=7.3.9",
    "@angular/compiler-cli": "=7.2.15",
    "@angular/language-service": "=7.2.15",
    "@ngrx/schematics": "=7.4.0",
    "@types/jest": "=23.3.14",
    "@types/lodash": "=4.14.125",
    "@types/node": "=8.10.36",
    "angular2-template-loader": "=0.6.2",
    "browser-sync": "=2.26.5",
    "codelyzer": "=4.5.0",
    "istanbul-reports": "=2.2.5",
    "jest-marbles": "2.3.1",
    "jest": "=24.8.0",
    "jest-cli": "=24.1.0",
    "jest-preset-angular": "=6.0.2",
    "jest-sonar-reporter": "=2.0.0",
    "jest-teamcity-reporter": "=0.9.0",
    "json-loader": "=0.5.7",
    "ngx-wallaby-jest": "=0.0.2",
    "protractor": "=5.4.1",
    "rxjs-tslint-rules": "=4.23.1",
    "ts-node": "=7.0.1",
    "tslint": "=5.16.0",
    "tslint-eslint-rules": "=5.4.0",
    "tslint-microsoft-contrib": "=6.1.1",
    "tslint-teamcity-reporter": "=3.2.2",
    "typescript": "=3.2.4",
    "wallaby-webpack": "=3.9.15"
  },
  "jest": {
    "preset": "jest-preset-angular",
    "moduleNameMapper": {
      "@core/(.*)": "<rootDir>/src/app/core/$1",
      "@shared/(.*)": "<rootDir>/src/app/shared/$1",
      "@views/(.*)": "<rootDir>/src/app/views/$1",
      "@app/(.*)": "<rootDir>/src/app/$1",
      "environments/(.*)": "<rootDir>/src/environments/$1"
    },
    "setupFilesAfterEnv": [
      "<rootDir>/src/setupJest.ts"
    ],
    "testResultsProcessor": "jest-teamcity-reporter"
  }
}
felikf commented 5 years ago

npm ls

...
+-- protractor@5.4.1
| +-- @types/node@6.14.0
| +-- @types/q@0.0.32
| +-- @types/selenium-webdriver@3.0.12
| +-- blocking-proxy@1.0.1
| | `-- minimist@1.2.0
| +-- browserstack@1.5.1
| | `-- https-proxy-agent@2.2.1
| |   +-- agent-base@4.2.1
| |   | `-- es6-promisify@5.0.0
| |   |   `-- es6-promise@4.2.5
| |   `-- debug@3.2.6
| |     `-- ms@2.1.1
| +-- chalk@1.1.3 deduped
| +-- glob@7.1.2 deduped
| +-- jasmine@2.8.0                                   <----------------- here jasmine
| | +-- exit@0.1.2 deduped
| | +-- glob@7.1.2 deduped
| | `-- jasmine-core@2.8.0
| +-- jasminewd2@2.2.0
...
just-jeb commented 5 years ago

If you create a simple test case with no marbles but with spyOn does it work? If it doesn't what happens when you remove the jest-marbles package? I'm asking because jest-marbles is an extension to jest matcher and it shouldn't affect spyOn in any way...

MatissJanis commented 5 years ago

I'm experiencing the same issue.

just-jeb commented 5 years ago

Can you create a minimal reproduction please?

MatissJanis commented 5 years ago

I actually failed creating a reproduction. It seems to work just fine in a fresh project. However in my existing project the call stack seems to be different. It feels like the test is being finished before the observable is initialised.

Given this test:

let consoleLog;

beforeEach(() => {
  consoleLog = jest.spyOn(console, 'log').mockImplementation();
});

afterEach(() => {
  console.warn('restore');
  consoleLog.mockRestore();
});

it('logs to console', () => {
  const input = new GetTouchpointSuccess({ } as any);

  actions.next(input);

  expect(effects.doSomething$).toBeObservable(cold('a', { a: input }));
  expect(consoleLog).toBeCalled();
  console.warn('end');
});

The output with jasmine-marbles is

 PASS  src/app/state/custom.effects.spec.ts
  CustomEffects
    .doSomething$
      ✓ logs to console (15ms)

  console.error src/app/state/custom.effects.ts:12
    call

  console.warn src/app/state/custom.effects.spec.ts:35
    end

  console.warn src/app/state/custom.effects.spec.ts:24
    restore

However as soon as I switch over to jest-marbles the call stack is:

 FAIL  src/app/state/custom.effects.spec.ts
  CustomEffects
    .doSomething$
      ✕ logs to console (16ms)

  ● CustomEffects › .doSomething$ › logs to console

    expect(jest.fn()).toBeCalled()

    Expected mock function to have been called, but it was not called.

      32 |
      33 |       expect(effects.doSomething$).toBeObservable(cold('a', { a: input }));
    > 34 |       expect(consoleLog).toBeCalled();
         |                          ^
      35 |       console.warn('end');
      36 |     });
      37 |   });

      at src/app/state/custom.effects.spec.ts:34:26
      at ZoneDelegate.Object.<anonymous>.ZoneDelegate.invoke (node_modules/zone.js/dist/zone.js:391:26)
      at ProxyZoneSpec.Object.<anonymous>.ProxyZoneSpec.onInvoke (node_modules/zone.js/dist/proxy.js:129:39)
      at ZoneDelegate.Object.<anonymous>.ZoneDelegate.invoke (node_modules/zone.js/dist/zone.js:390:52)
      at Zone.Object.<anonymous>.Zone.run (node_modules/zone.js/dist/zone.js:150:43)
      at Object.<anonymous> (node_modules/jest-preset-angular/zone-patch/index.js:52:27)

  console.warn src/app/state/custom.effects.spec.ts:24
    restore

  console.error src/app/state/custom.effects.ts:12
    call

  console.log src/app/state/custom.effects.ts:13
    This should not appear in tests
hanvyj commented 5 years ago

I've also been seeing this. I've actually been just avoiding using expect.toHaveBeenCalledWith assuming it was a limitation of jest-marbles, but I just switched to jasmine-marbles and it works fine!

Jest:

FAIL src/pages/Workbench/epics/list-users.test.ts (51.932s) epics × a list users call is fired when the location changes (35ms)

● epics › a list users call is fired when the location changes

expect(jest.fn()).toHaveBeenCalledWith(expected)

Expected mock function to have been called with:
  ["organisation-123"]
But it was not called.

  88 |     // Assert
  89 |     expect(output$).toBeObservable(listUsers$);
> 90 |     expect(mockListUsersAction).toHaveBeenCalledWith("organisation-123");
     |                                 ^
  91 |   });
  92 | });

After trying to find out any work-around I stumbled upon this and using jasmine-marbles works fine!

just-jeb commented 5 years ago

@hanvyj would you mind providing a minimal reproduction repo? If you can also recreate the same repo but with jasmine-marbles it would be even better!

just-jeb commented 5 years ago

The problem is that as I mentioned previously, the test scheduler is flushed after each test in jest-marbles. Therefore effectively the observables run after the test is executed.
In jasmine-marbles the scheduler is flushed after each expect. In my opinion this is wrong, because when you do multiple assertions within a single test only the first one will pass, the others will fail because of the messed time frame. Thus, such a trivial test would fail in jasmine-marbles and pass in jest-marbles:

  it("Should pass the two expects", () => {
    const hotInput = hot("  ---^--a|");
    const coldInput = cold("---a|");
    const expected = cold("---a|");

    expect(hotInput).toBeObservable(expected);
    expect(coldInput).toBeObservable(expected);
  });

I would recommend you to test your code in a way that you could assert on the output stream. However, if you have to assert on a side effect you could use this workaround.

just-jeb commented 5 years ago

Please use toSatisfyOnFlush, available since 2.5.0.
Let me know if it solves your problem and the issue can be closed.

henriquezago commented 4 years ago

Please use toSatisfyOnFlush, available since 2.5.0. Let me know if it solves your problem and the issue can be closed.

sorry but, why?? That solves my problem, but it makes it looks terrible or am I doing something wrong? from:

expect(serviceMock.getDocument).toHaveBeenCalledWith(suppliedDocumentIds, hash);

to:

expect(expected.pipe(tap(ossServiceMock.getDocument))).toSatisfyOnFlush(() => {
    expect(serviceMock.getDocument).toHaveBeenCalledWith(suppliedDocumentIds, hash);
});
mcblum commented 4 years ago

Does anyone have any updates on this? I'm seeing the same issue and it's a pretty serious limitation to using jest-marbles.

just-jeb commented 4 years ago

@mcblum Does satisfyOnFlush work for you?

just-jeb commented 4 years ago

@henriquezago I explained the reasoning here.
I think it's better to have a special matcher for side effects rather than making a simple test case (like in the example) fail.

mcblum commented 4 years ago

@just-jeb I can give it a try but TBH the syntax is really confusing and on first glance feels like it clutters up the code base a ton and makes it difficult for future developers to know what's happening.

just-jeb commented 4 years ago

@mcblum I agree the syntax is confusing, but I think it's better to have a confusing syntax for edge cases rather than having fully legitimate test flow to fail, don't you think?
I'll try to come up with a better solution, but for the meanwhile working and confusing is better than not working and clear.

zoechi commented 3 years ago

toSatisfyOnFlush solved it for me but it took several hours until I figured out why toHaveBeenCalled...() didn't work. The README.md mentions it, but it took a while until I realized that this is related.