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

mock.onGet() matches URL with trailing slash #136

Open
souldzin opened this issue May 9, 2018 · 4 comments
Open

mock.onGet() matches URL with trailing slash #136

souldzin opened this issue May 9, 2018 · 4 comments

Comments

@souldzin
Copy link

souldzin commented May 9, 2018

What is the current behavior?

In the code below, the "mock.onGet without trailing slash" test fails.

import axios from 'axios';
import MockAdapter from 'axios-mock-adapter'

class Foo {
  constructor(endpoint) {
    this.axios = axios.create({
      baseURL: endpoint
    });
  }

  getFoos() {
    return this.axios.get();
  }

  getFooBars() {
    return this.axios.get("bar");
  }
}

describe("Foo", () => {
  const ENDPOINT = "http://test.com/bogus"
  let mock;
  let subject;

  beforeEach(() => {
      mock = new MockAdapter(axios);
      subject = new Foo(ENDPOINT);
  })

  afterEach(() => {
    mock.restore();
  });
  
  // ----- This test fails :( ---------------------
  it("mock.onGet without trailing slash", () => {
    mock.onGet(ENDPOINT).reply(200, {});

    return subject.getFoos();
  });  

  // ----- This one passes O_o --------------------
  it("mock.onGet with trailing slash", () => {
    mock.onGet(`${ENDPOINT}/`).reply(200, {});

    return subject.getFoos();
  })
});

What is expected?

  • I expect the "mock.onGet without trailing slash" test to pass.
  • I'm not sure if the "mock.onGet with trailing slash" should pass or not... I would think not...

What is causing this?

The current implementation for combineUrls() leaves a trailing slash when url is empty. (See utils.js for more info).

I would expect this to be consistent with the way axios combines URLs. Maybe we can even import this method?

What am I willing to do about it?

I can create a PR that updates the implementation of combineUrls(), if it is agreed that this is an issue.

@javierguzman
Copy link

I am having a similar issue. For me even adding a trailing slash does not work. However if I put the url as regular expresion with /* at the end it works.

@babacarcissedia
Copy link

I think this should be closed since we can achieve the result by making the a regex .*

@javierguzman
Copy link

@bcdbuddy I am not a maintainer but in my opinion because we can do it with regex does not mean is enough for closing the issue. I mean, it should work without regex, shouldn't it?

@babacarcissedia
Copy link

I don't think so. I am assuming it is only doing string comparison so if there is a trailing slash or not that can cause a mismatch. Simple enough. As long as there is one way of achieving the expected result without any change on this lib I think we should go for that option.
In any case, this repo seems dead

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants