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.

Sign up for GitHub

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

Jump to bottom

chore: convert API tests to node test #1498

Merged
merged 4 commits into from
Sep 11, 2024
Merged

chore: convert API tests to node test #1498

merged 4 commits into from
Sep 11, 2024

Conversation

jkaster
Copy link
Contributor

@jkaster jkaster commented Sep 11, 2024

Convert tests that call the Looker API server to use node test runner

Test files renamed from *.spec.ts to *.apitest.ts

Only files with .apitest use the node test runner

@jkaster jkaster requested a review from a team as a code owner September 11, 2024 00:49
@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Sep 11, 2024 •
edited
Loading

APIX Tests

0 tests  ±0   0 ✅ ±0   0s ⏱️ ±0s
0 suites ±0   0 💤 ±0 
0 files   ±0   0 ❌ ±0 

Results for commit 733c9a7. ± Comparison against base commit 748621e.

♻️ This comment has been updated with latest results.

@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Sep 11, 2024 •
edited
Loading

Codegen Tests

439 tests   - 20   423 ✅ ± 0   36s ⏱️ -2s
 19 suites  -  2    16 💤  - 20 
  1 files   ± 0     0 ❌ ± 0 

Results for commit 733c9a7. ± Comparison against base commit 748621e.

This pull request removes 20 tests. 
fetch operations can login ‑ fetch operations can login
fetch operations defaults lookerVersion when server is not responding ‑ fetch operations defaults lookerVersion when server is not responding
fetch operations get versions ‑ fetch operations get versions
fetch operations gets lookerVersion with good server ‑ fetch operations gets lookerVersion with good server
fetch operations gets lookerVersion with supplied versions ‑ fetch operations gets lookerVersion with supplied versions
fetch operations gets version info ‑ fetch operations gets version info
fetch operations logConvertSpec ‑ fetch operations logConvertSpec
spec conversion collectionFormat to style ‑ spec conversion collectionFormat to style
spec conversion fixes missing conversion items ‑ spec conversion fixes missing conversion items
spec conversion getSpecsFromVersions current is the default spec ‑ spec conversion getSpecsFromVersions current is the default spec
…

♻️ This comment has been updated with latest results.

@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Sep 11, 2024 •
edited
Loading

Typescript Tests

  1 files  ±0   28 suites  ±0   25s ⏱️ -1s
208 tests ±0  207 ✅ ±0  1 💤 ±0  0 ❌ ±0 
222 runs  ±0  221 ✅ ±0  1 💤 ±0  0 ❌ ±0 

Results for commit 733c9a7. ± Comparison against base commit 748621e.

♻️ This comment has been updated with latest results.

josephaxisa
josephaxisa previously approved these changes Sep 11, 2024
View reviewed changes
Copy link
Contributor

@josephaxisa josephaxisa left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@jkaster jkaster requested a review from josephaxisa September 11, 2024 22:43
bryans99
bryans99 approved these changes Sep 11, 2024
View reviewed changes
Copy link
Collaborator

@bryans99 bryans99 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Questions and minor nits but LGTM

packages/sdk-node/src/nodeSession.apitest.ts
import { ApiConfigMap, boolDefault, defaultTimeout } from '@looker/sdk-rtl';
import { TestConfig } from '@looker/sdk-codegen-utils';
import { TestConfig } from '@looker/sdk-rtl/src/testUtils';
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need to reach into the package?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, unfortunately

packages/sdk-node/test/sdk.apitest.ts
@@ -423,11 +420,11 @@ describe('LookerNodeSDK integration tests', () => {
});

describe('User CRUD-it checks', () => {
beforeAll(async () => {
before(async () => {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This gets run before every test?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no, that's the node test runner beforeAll

packages/sdk-node/test/sdk.apitest.ts Show resolved Hide resolved
packages/sdk-node/test/sdk.apitest.ts Show resolved Hide resolved
@jkaster jkaster merged commit d2670ae into main Sep 11, 2024
22 checks passed
@jkaster jkaster deleted the tests_for_node branch September 11, 2024 23:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bryans99 bryans99 bryans99 approved these changes

@treshugart treshugart Awaiting requested review from treshugart

@prescottbreeden prescottbreeden Awaiting requested review from prescottbreeden

@josephaxisa josephaxisa Awaiting requested review from josephaxisa

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@jkaster @bryans99 @josephaxisa