Skip to content

Bug with location.back() #280

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

Closed
meirka opened this issue Feb 5, 2022 · 14 comments · Fixed by #286
Closed

Bug with location.back() #280

meirka opened this issue Feb 5, 2022 · 14 comments · Fixed by #286
Labels
bug Something isn't working released

Comments

@meirka
Copy link
Contributor

meirka commented Feb 5, 2022

Hello, I believe I found a bug, where angular testing library doesn't seem to handle location.back() scenario.

I have prepared simple project with very basic navigation, specs with TestBed and with Angular Testing Library. In case of TestBed navigation works correctly, but with Angular Testing Library it does not.

https://github.com/meirka/angular-unit-test-navigation

I have similar test in my side project and i'm experiencing the same issue.

Please advise.

Thank you.

@timdeschryver
Copy link
Member

Hi, this seems like to be a bug.
I've opened a PR to your example (meirka/angular-unit-test-navigation#2) with a temporary fix.

Since it's the first time that I've encountered the initialNavigation method, I would like to take a closer at it. My current thought is that invoking this method on render becomes the default behavior of ATL when the routermodule is imported. But I don't want to rush it 😅

If you have any ideas, feel free to suggest them.
And if you want feel free to reproduce this bug within this repository as that would help me.
Also, if you want, you can could also implement the fix.

@timdeschryver timdeschryver added the bug Something isn't working label Feb 7, 2022
@meirka
Copy link
Contributor Author

meirka commented Feb 7, 2022

@timdeschryver, I can write the test, are you sure you want me to put test in?
I never done stuff like this.

As per fix, no no no... I don't understand how Angular or Angular Test Framework works, I definitely don't want to mess with it.

Thank you for the help. I'll make PR (with test) in next couple of days.

P.S: developing angular app is just a hobby, I don't work with it on daily basis. I just spend an hour or so on weekends.

@meirka
Copy link
Contributor Author

meirka commented Feb 7, 2022

Cloned project and tests are failing before I even started...

@timdeschryver
Copy link
Member

@meirka Oh.. yikes those failing tests shouldn't happen 😅
Also, I don't want to obligate you - if you don't feel like it, I'm happy to pick this up or to help you.

@meirka
Copy link
Contributor Author

meirka commented Feb 9, 2022

@timdeschryver I would like to add a test. I emailed you test error details. I figured to take test issues off the thread.

@timdeschryver
Copy link
Member

@meirka Oh, I think that you mailed it to a no-reply address... perhaps that I should use my "real" e-mail address here 😅

I'll try to run the tests locally and see if I can reproduce the error.
If that's not the case, then I'll ping you here to get the error message (if you want, you can also add the error message in this thread)

@timdeschryver
Copy link
Member

@meirka I can confirm that I also have the errors, seems like some newer version TL is causing this.
I'll take a look at it over the next days.
Sorry for the troubles.

@timdeschryver
Copy link
Member

@meirka the error should be resolved with #282 - sorry for the inconvenience.

@meirka
Copy link
Contributor Author

meirka commented Feb 10, 2022

@timdeschryver Yeah, tests are good now. Give me few days, let me try to workout tests

@meirka
Copy link
Contributor Author

meirka commented Feb 11, 2022

@timdeschryver
Can you help me a bit? I got stuck with the test:
https://github.com/meirka/angular-testing-library

It doesn't want to click on 'navigate back' and I don't understand why.

@timdeschryver
Copy link
Member

@meirka try this.
You have to add the components to the declarations, also you should away the findBy queries.

The following always is truthy because findBy returns a promise.

await expect(screen.findBy()).toBeTruthy

The version with the declarations:

@Component({
  template: `<div>Navigate</div>
    <router-outlet></router-outlet>`,
})
class MainComponent {}

@Component({
  template: `<div>first page</div>
    <a routerLink="/second">go to second</a>`,
})
class FirstComponent {}

@Component({
  template: `<div>second page</div>
    <button (click)="goBack()">navigate back</button>`,
})
class SecondComponent {
  constructor(private location: Location) {}
  goBack() {
    this.location.back();
  }
}

const routes: Routes = [
  { path: '', redirectTo: '/first', pathMatch: 'full' },
  { path: 'first', component: FirstComponent },
  { path: 'second', component: SecondComponent },
];

@NgModule({
  declarations: [FirstComponent, SecondComponent],
  imports: [RouterModule.forRoot(routes)],
  exports: [RouterModule],
})
class AppRoutingModule {}

test('navigate', async () => {
  // await render(MainComponent, {imports: [AppRoutingModule, RouterTestingModule]});
  const subject = await render(MainComponent, { imports: [AppRoutingModule, RouterTestingModule] });
  await subject.navigate('/');

  const router = TestBed.inject(Router);
  router.initialNavigation();

  expect(await screen.findByText('Navigate')).toBeTruthy();
  expect(await screen.findByText('first page')).toBeTruthy();

  click(await screen.findByText('go to second'));

  expect(await screen.findByText('second page')).toBeTruthy();
  expect(await screen.findByText('navigate back')).toBeTruthy();

  click(await screen.findByText('navigate back', { selector: 'button' }));

  expect(await screen.findByText('first page')).toBeTruthy();
});

@meirka
Copy link
Contributor Author

meirka commented Feb 12, 2022

Thank you!

@meirka
Copy link
Contributor Author

meirka commented Feb 12, 2022

@timdeschryver PR is up - made changes, verified that test is failing (due to the bug).
Let me know what else I can do.
Also please add me to the pull request for the fix, I would like to read over it.
Thank you!

@github-actions
Copy link

🎉 This issue has been resolved in version 11.0.3 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working released
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants