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

Prettier Issue and Existing Test Case Failure #61

Open
md-abid-hussain opened this issue Oct 17, 2024 · 1 comment
Open

Prettier Issue and Existing Test Case Failure #61

md-abid-hussain opened this issue Oct 17, 2024 · 1 comment

Comments

@md-abid-hussain
Copy link
Contributor

md-abid-hussain commented Oct 17, 2024

Prettier issue
image

and failing test cases
image

@md-abid-hussain
Copy link
Contributor Author

md-abid-hussain commented Oct 17, 2024

The fix is simple

The prettier issue is caused by a redundant import in src/models/modelsRestApiClient.ts at line 14. This is also causing 4 test case failures.

The one test case is failing due to logic error in the connect method, which is inside src/index.ts

const connect = async function (options: ConnectionOptions): Promise<void> {
  ...
  const baseURL =
    httpClient.defaults.baseURL || Constants.BASE_CLOUD_API_ENDPOINT;
  // Need to authenticate if we're using the Cloud API endpoints.
  if (isMindsDbCloudEndpoint(baseURL) || !isLocalEndpoint(baseURL)) {
    ...
  }
};

isMindsDbCloudEndpoint method

function isMindsDbCloudEndpoint(url: string): boolean {
  // Cloud endpoints:
  // - https://cloud.mindsdb.com
  // - https://alpha.mindsdb.com
  // - https://beta.mindsdb.com
  return url.includes('mindsdb.com');
}

isLocalEndpoint

function isLocalEndpoint(url: string): boolean {
  return url.includes('localhost') || url.includes('127.0.0.1');
}

And test case is

test('connect should not authenticate for custom endpoint', async () => {
    await MindsDB.connect({
      host: 'https://test-url.com',
      user: 'test-user',
      password: 'test-password',
      httpClient: mockedAxios,
    });
    expect(mockedAxios.post).not.toHaveBeenCalled();
  });

https://test-url.com will give true for

if (isMindsDbCloudEndpoint(baseURL) || !isLocalEndpoint(baseURL))

and hence test case will fail

@md-abid-hussain md-abid-hussain changed the title Existing Test Case Failure Prettier Issue and Existing Test Case Failure Oct 29, 2024
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 a pull request may close this issue.

1 participant