[META] fork / upstream maintenance discussion

[cspotcode]

trafficstars

In NagRock/ts-mockito/issues/212

There is discussion about adding maintainers to the original ts-mockito project.

I still hope the author, NagRock, is able to add maintainers. Barring that, we may want to collaborate on this fork: https://npmjs.com/package/@johanblumenberg/ts-mockito

I also have access to the @TypeStrong org, and we can consider moving the project there.

---

Other links:

This fork needs passing tests, being discussed in #2

[LironHazan]

All tests are passing now, the node version on git workflow was updated to v14.15.5 Future improvements IMO:

• Configure prettier + eslint instead of tslint, we can also
• Can try to get a better score --> https://snyk.io/advisor/npm-package/ts-mockito seek for vulnerabilities and resolve them, improve the documentation etc..

[LironHazan]

Should open a PR for merging #194 Fixed problem with a mock resolving a mocked value with Promises by @jlkeesey Seems there are 2 discussions around it: #191 #219 The PR was approved but never merged @cspotcode what is the acceptable approach of porting PR's opened for the original repo into the forked repo? Not sure it's possible to access the #194 commits from the forked repo, will just copying and leaving a comment with the issue id near the fix be OK?

[cspotcode]

Not sure it's possible to access the #194 commits from the forked repo, will just copying and leaving a comment with the issue id near the fix be OK?

The pull request creation UI should allow creating a pull request from any branch to any branch. Once I pick the correct forks and branches in the dropdowns, it appears to show me the correct commits.

[cspotcode]

I created PR #5 for this.

[LironHazan]

@cspotcode awesome! thanks I'll try doing that myself with another PR

[LironHazan]

Ported a feature PR #140 mock free functions

[LironHazan]

Ported #139 Matcher types

[cspotcode]

I noticed that the johanblumenberg README has a list of features it adds which are not in the upstream library: https://github.com/johanblumenberg/ts-mockito#johanblumenbergts-mockito

Maybe we can try to port all of those features.

[cspotcode]

I sent a support ticket to npm support requesting a transfer of the @typestrong scope. If we want, and if they approve the transfer, we can publish ts-mockito under the @typestrong/ts-mockito scope.

[LironHazan]

@cspotcode I know I already opened PR's for porting features from @johanblumenberg but come to think of it after reading the latest comments on the maintenance thread I realized that maybe it would be better to resolve un-addressed "pains" - issues first and have a clear agenda of how it would be maintained outside of the original repo in case the author wouldn't intend to merge the fork into the original repo, I understand that most of the participants of the thread would like to see the original repo maintained but I have a lack of faith in personal repos in situation like those and IMO a heavily used project should have a community with a clear code of conduct, agenda, skilled developers willing to contribute - saying this carefully, as I do respect the work that was done by the author and others, I just feel the thread is moving in circles.. If I'll see we're making a progress with @typestrong/ts-mockito I'll start using that..

[cspotcode]

I like moving to TypeStrong. Github will let me transfer this repository to TypeStrong today. Is there more to discuss, or are we ready to do that right now?

[LironHazan]

@cspotcode Let's rock \m/

[cspotcode]

Transfer complete. You should still have full access. If the permissions got dropped for some reason, let me know and I'll be sure to fix it.

[LironHazan]

@cspotcode cool I see I have access, I'll try to find the time to replace tslint in eslint after my working hours and maybe go over the issues see if there's some bugs that still happen that can be resolved. LMK when you'll publish to npm, worth adding a badge to the readme.

[cspotcode]

@LironHazan where do you want to publish in the short-term? Ideally, we publish to npm ts-mockito if and when NagRock grants us access. Until then, should we publish to:

• @cspotcode/ts-mockito
• @typestrong/ts-mockito
• @ts-mockito/ts-mockito

I am still waiting to hear from npm support about the @typestrong scope. I am also not sure if organizations have granular permissions. We would want to be sure ts-mockito maintainers have access to publish @typestrong/ts-mockito, but not to publish unrelated typestrong packages with different maintainership teams.

I have successfully reserved the @ts-mockito scope.

[LironHazan]

so if it won't be easy/possible to publish into @typestrong/ts-mockito we can go with @ts-mockito/ts-mockito IMO @cspotcode

[cspotcode]

I did some more googling and finally found what I was looking for. https://docs.npmjs.com/managing-team-access-to-organization-packages npm will let us create multiple teams inside an organization, each with access to their own packages. So we can use @typestrong/ts-mockito if/when npm support replies to me. I'll wait a bit longer for them to reply.

[cspotcode]

I did not receive a response from npm for my initial support request; I've sent another. I've included a copy below.

If you would prefer to move ahead publishing to @ts-mockito/ts-mockito, I can invite you to the organization on npm. This will grant you permission to publish.

Type: account or billing issue

Subject: Requesting transfer of parked @typestrong scope

We would like to publish @typestrong/ts-mockito from https://github.com/TypeStrong/ts-mockito. I see the @typestrong organization is owned by someone else but does not have any published packages. Can we request a transfer of the organization? Is this sufficient information, or do you need anything else? How long does it typically take for a request like this to reach a resolution?

I cannot find contact info for the owner of @typestrong, but is there a way I can reach them to discuss a transfer?

I am following the instructions from https://docs.npmjs.com/policies/disputes

Thanks in advance.

[LironHazan]

We can use @ts-mockito/ts-mockito I don't mind, bTW I started to gradually configure eslint, the default linter is still tslint but if you'll configure your code-editor to detect the eslint config you'll start getting errors, running eslint will produce: 422 problems (357 errors, 65 warnings) most of them are of: no-unsafe-assignment no-unsafe-return no-unsafe-call no-unsafe-member-access As a result of a massive use of any everywhere LOL, honestly I'm not sure if I'll keep fixing types, won't worth the effort..

[cspotcode]

I would avoid making any changes that are likely to cause merge conflicts with code or pull requests already written for either of the other 2 ts-mockito repositories. Better to be able to cleanly merge pre-existing features and bugfixes.

Why is your npm username? I'll add it to the organization.

On Sat, Oct 9, 2021, 7:44 AM LironH @.***> wrote:

We can use @ts-mockito/ts-mockito I don't mind, bTW I started to gradually configure eslint, the default linter is still tslint but if you'll configure your code-editor to detect the eslint config you'll start getting errors, running eslint will produce: 422 problems (357 errors, 65 warnings) most of them are of: no-unsafe-assignment no-unsafe-return no-unsafe-call no-unsafe-member-access As a result of a massive use of any everywhere LOL, honestly I'm not sure if I'll keep fixing types, won't worth the effort..

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/TypeStrong/ts-mockito/issues/3#issuecomment-939283288, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAC35OHZDROOCLAJ5FHQM2LUGATKHANCNFSM5D3ECXSQ .

[LironHazan]

@cspotcode lironhazan thanks

[cspotcode]

How do you want to handle changelogs?

I don't have a strong opinion. For ts-node we use Github Releases, (example) and I've recently started adding issues and pull requests to "milestones" to help me create the release notes.

[LironHazan]

I don't mind using Github Releases, I've never really managed any changelogs before, I've noticed that some projects uses a CHANGELOG.md but I can't tell what's better

[cspotcode]

Let's use Github Releases since NagRock already does: https://github.com/NagRock/ts-mockito/releases

The big things that I personally try to avoid are:

• automatically generating changelogs from commit logs. This requires commit logs to be too perfect. Commits can get messy; sometimes multiple PRs implement a single feature. Better to write a changelog by hand after a release is completed, IMO
• Immutable changelogs. I don't publish the changelog to npm inside the package. If we make a mistake, we should be able to fix it.

Both Github Releases and CHANGELOG.md let us make corrections and amendments to the changelog at any time, which is a good thing.

[cspotcode]

I created #11 which renames from @cspotcode to @ts-mockito so that we can publish to npm.

I also created #12 where we should audit every upstream commit that does not exist in this fork. If there is anything important that we missed, we should find a way to get it merged into this fork.

[LironHazan]

12 is great I see there're conflicts, I'll review when possible

[cspotcode]

It took a little while, but npm transferred the @typestrong scope to me. I'm on vacation this weekend, but I can publish an initial release of @typestrong/ts-mockito when I'm back.

[pauleustice]

@cspotcode Do you have still plan to release this to npm? Or is there another way I can consume it? The original ts-mockito stopped working with Typescript 4.4.2 and I'd like to see if your fork resolves the issue at all.

FYI the issue seems to be that const serviceSpy = spy(someAngularService) (when someAngularService is the service being tested, and not a provided dependency) replaces that service's observables with mock functions (only when they are formed from combineLatest or possibly other RxJS operators). This obviously breaks all the tests.

I created a test repo here

EDIT: Also seems to occur for spying on components.

e.g.

// somecomponent.component.ts

export class SomeComponent {
  someObservable$: Observable<someInterface> = someObservableFromSomewhereElse;
}

// somecomponent.component.spec.ts

describe('SomeComponent', () => {
  beforeEach(() => {
    TestBed.configureTestingModule({
      providers: [],
    }).compileComponents();

    fixture = TestBed.createComponent(SomeComponent);
    component = fixture.componentInstance;

    spyComponent = spy(component);
  })
  
  it('should have an observable', () => {
    console.log(component.someObservable$.toString())
  
    //  Logs out ->
    
    // function () {
    //   var args = [];
    //   for (var _i = 0; _i < arguments.length; _i++) {
    //     args[_i] = arguments[_i];
    //   }
    //   var action = new MethodAction_1.MethodAction(key, args);
    //   _this.methodActions.push(action);
    //   var methodStub = _this.getMethodStub(key, args);
    //   methodStub.execute(args);
    //   return methodStub.getValue();
    // }
  });
}

[LironHazan]

@pauleustice Hey I replied on the other thread that our tests which use tsmockito works fine but we're actually not using TestBed, and since we're running with Jest we're also using the jest.spyOn method:

Here's an example of instantiation without the TestBed, the mocked instances are directly injected,

describe(' test query editor', () => {
  let component: QueryEditorComponent;
  const mockedFeaturesFlagService = mock(FeatureFlagsService);
  const mockedS1qlLangService = mock(S1qlLangService);
  const mockedPQLLangService = mock(PowerQueryLangService);
  const mockedQueryEditorService = mock(QueryEditorService);
  const mockedCdr = mock(ChangeDetectorRef);

  beforeEach(() => {
    component = new QueryEditorComponent(
      instance(mockedS1qlLangService),
      instance(mockedPQLLangService),
      instance(mockedQueryEditorService),
      instance(mockedFeaturesFlagService),
      instance(mockedCdr)
    );
  });
  
    it('should emit validation state when invoking updateValidationState', () => {
    jest.spyOn(component.validationState, 'emit');
    component.updateValidationState(true);
    expect(component.validationState.emit).toHaveBeenCalledTimes(1);
    expect(component.validationState.emit).toHaveBeenCalledWith({ isValid: true });
  });

I guess @cspotcode didn't publish to npm yet

[pauleustice]

@LironHazan Thanks for both your replies :) I looked at replacing the ts-mockito spy() with jest's spyOn (we're also using Jest), and while it worked, there were 2 problems:

• We have thousands of tests, so it's not an option right now
• It doesn't offer a combination of toHaveBeenCalledTimes and toHaveBeenCalledWith, which you get with ts-mockito
  • This would mean a reduction in our test quality

e.g.

ts-mockito:

verify(serviceSpy.someMethod('foo')).times(3);

jest:

expect(serviceSpy.someMethod). toHaveBeenCalledWith('foo');
expect(serviceSpy.someMethod). toHaveBeenCalledTimes(3);

In Jest, you can't tell that foo was passed three times, just that it was passed and the method was called (with potentially anything) three times. It's a small distinction, but something we use a lot in our test suites.