Skip to content

Conversation

@drstrangelooker
Copy link
Collaborator

No description provided.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@drstrangelooker drstrangelooker force-pushed the drstrangelove/release-21.20 branch from f80a8c3 to 65de88a Compare November 9, 2021 17:56
@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@drstrangelooker drstrangelooker force-pushed the drstrangelove/release-21.20 branch from 65de88a to 49277c1 Compare November 9, 2021 18:04
@github-actions
Copy link
Contributor

github-actions bot commented Nov 9, 2021

Python Tests

0 files  0 suites   0s ⏱️
0 tests 0 ✔️ 0 💤 0 ❌

Results for commit 49277c1.

@github-actions
Copy link
Contributor

github-actions bot commented Nov 9, 2021

Typescript Tests

    6 files    75 suites   3m 49s ⏱️
167 tests 163 ✔️   4 💤 0 ❌
555 runs  543 ✔️ 12 💤 0 ❌

Results for commit 49277c1.

…ariable called 'models'. The names interfered. I added the name models to the list of keywords and modified some methods to the proper arguments were generated with 'models_' instead.
@github-actions
Copy link
Contributor

github-actions bot commented Nov 9, 2021

Python Tests

    8 files      8 suites   1m 35s ⏱️
140 tests 137 ✔️ 0 💤 3 ❌
742 runs  736 ✔️ 0 💤 6 ❌

For more details on these failures, see this check.

Results for commit 1d085d4.

@github-actions
Copy link
Contributor

github-actions bot commented Nov 9, 2021

Codegen Tests

  1 files    6 suites   22s ⏱️
58 tests 46 ✔️ 12 💤 0 ❌

Results for commit 1d085d4.

@github-actions
Copy link
Contributor

github-actions bot commented Nov 9, 2021

Typescript Tests

    6 files    75 suites   3m 54s ⏱️
167 tests 163 ✔️   4 💤 0 ❌
555 runs  543 ✔️ 12 💤 0 ❌

Results for commit 1d085d4.

Copy link
Contributor

@rollyjoel rollyjoel left a comment

Choose a reason for hiding this comment

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

what do you think about just changing the import inside methods.py to something like

from . import models as mdls

@github-actions
Copy link
Contributor

github-actions bot commented Nov 9, 2021

Python Tests

    9 files      9 suites   1m 55s ⏱️
140 tests 137 ✔️ 0 💤 3 ❌
765 runs  756 ✔️ 0 💤 9 ❌

For more details on these failures, see this check.

Results for commit dda374e.

@github-actions
Copy link
Contributor

github-actions bot commented Nov 9, 2021

Codegen Tests

  1 files    6 suites   20s ⏱️
58 tests 46 ✔️ 12 💤 0 ❌

Results for commit dda374e.

@github-actions
Copy link
Contributor

github-actions bot commented Nov 9, 2021

Python Tests

    7 files      7 suites   41s ⏱️
140 tests 137 ✔️ 0 💤 3 ❌
719 runs  716 ✔️ 0 💤 3 ❌

For more details on these failures, see this check.

Results for commit 94a6f63.

@rollyjoel rollyjoel self-requested a review November 9, 2021 22:16
rollyjoel
rollyjoel previously approved these changes Nov 9, 2021
Copy link
Contributor

@rollyjoel rollyjoel left a comment

Choose a reason for hiding this comment

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

not pretty but I think that's the best we'll do for python. I think you might need to update some tests but we'll find out

@github-actions
Copy link
Contributor

github-actions bot commented Nov 9, 2021

Python Tests

    7 files      7 suites   42s ⏱️
140 tests 137 ✔️ 0 💤 3 ❌
719 runs  716 ✔️ 0 💤 3 ❌

For more details on these failures, see this check.

Results for commit d51789b.

@rollyjoel
Copy link
Contributor

@drstrangelooker looks like the look-id int -> string change needs to be updated in the python tests

@jkaster
Copy link

jkaster commented Nov 9, 2021

Joel correctly called out that python.gen.spec.ts (the test for python.gen.ts) needs to be updated and should also be part of the CI call. Interesting that yarn test:sdk isn't being invoked (evidently, because it should fail).

@rollyjoel
Copy link
Contributor

sorry, yeah what I mean is the specific language generator tests under packages/sdk-codegen/src/ seem to not be getting run as part of the build

@github-actions
Copy link
Contributor

github-actions bot commented Nov 9, 2021

Python Tests

  10 files    10 suites   2m 58s ⏱️
140 tests 140 ✔️ 0 💤 0 ❌
788 runs  788 ✔️ 0 💤 0 ❌

Results for commit aabb98d.

@github-actions
Copy link
Contributor

github-actions bot commented Nov 9, 2021

Codegen Tests

  1 files    6 suites   24s ⏱️
58 tests 46 ✔️ 12 💤 0 ❌

Results for commit aabb98d.

@github-actions
Copy link
Contributor

github-actions bot commented Nov 9, 2021

Typescript Tests

    7 files    76 suites   4m 52s ⏱️
167 tests 163 ✔️   4 💤 0 ❌
582 runs  566 ✔️ 16 💤 0 ❌

Results for commit aabb98d.

@drstrangelooker
Copy link
Collaborator Author

drstrangelooker commented Nov 10, 2021

I've been using bin/smoke and bin/unit_test. I've cleaned up the results of yarn test:sdk now.

@github-actions
Copy link
Contributor

Python Tests

  10 files    10 suites   3m 11s ⏱️
140 tests 140 ✔️ 0 💤 0 ❌
788 runs  788 ✔️ 0 💤 0 ❌

Results for commit 61e16b2.

jkaster
jkaster previously approved these changes Nov 10, 2021
Copy link

@jkaster jkaster left a comment

Choose a reason for hiding this comment

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

LGTM. I think this should pass CI.

@github-actions
Copy link
Contributor

Codegen Tests

  1 files    6 suites   27s ⏱️
58 tests 46 ✔️ 12 💤 0 ❌

Results for commit 61e16b2.

@drstrangelooker
Copy link
Collaborator Author

Found our issue. The tests are run by

yarn jest "packages/sdk-codegen(-utils|-scripts)/src" --reporters=default --reporters=jest-junit

Any tests in packages/sdk-codegen/src are skipped. Instead we need this expression:

yarn jest "packages/sdk-codegen(|-utils|-scripts)/src" --reporters=default --reporters=jest-junit

@github-actions
Copy link
Contributor

Typescript Tests

    7 files    76 suites   4m 54s ⏱️
167 tests 163 ✔️   4 💤 0 ❌
582 runs  566 ✔️ 16 💤 0 ❌

Results for commit 61e16b2.

@drstrangelooker
Copy link
Collaborator Author

drstrangelooker commented Nov 10, 2021

I think we should fix the workflows so that the .gen.spec.ts files get run, especially because I think at least python.gen.spec.ts is broken now? #899 (comment)

Should be resolved now.

This looks like such a weird pattern yarn jest "packages/sdk-codegen(|-utils|-scripts)/src". It works on my machine but I'm waiting to see if it works properly in CI.

edit: it worked fine.

$ /home/runner/work/sdk-codegen/sdk-codegen/node_modules/.bin/jest 'packages/sdk-codegen(|-utils|-scripts)/src' --reporters=default --reporters=jest-junit
PASS packages/sdk-codegen/src/sdkModels.spec.ts (5.403 s)
PASS packages/sdk-codegen/src/python.gen.spec.ts
PASS packages/sdk-codegen/src/typescript.gen.spec.ts
PASS packages/sdk-codegen-scripts/src/convert.spec.ts (10.767 s)
PASS packages/sdk-codegen/src/kotlin.gen.spec.ts
PASS packages/sdk-codegen/src/specDiff.spec.ts
PASS packages/sdk-codegen/src/csharp.gen.spec.ts
PASS packages/sdk-codegen-scripts/src/exampleMiner.spec.ts
PASS packages/sdk-codegen/src/swift.gen.spec.ts
PASS packages/sdk-codegen/src/pseudo.gen.spec.ts
PASS packages/sdk-codegen/src/go.gen.spec.ts
PASS packages/sdk-codegen/src/codeGenerators.spec.ts
PASS packages/sdk-codegen-scripts/src/fetchSpec.spec.ts (6.928 s)

@github-actions
Copy link
Contributor

Python Tests

  10 files    10 suites   3m 2s ⏱️
140 tests 140 ✔️ 0 💤 0 ❌
788 runs  788 ✔️ 0 💤 0 ❌

Results for commit 75298ac.

@github-actions
Copy link
Contributor

Codegen Tests

    1 files    17 suites   34s ⏱️
395 tests 381 ✔️ 14 💤 0 ❌
396 runs  382 ✔️ 14 💤 0 ❌

Results for commit 75298ac.

@drstrangelooker drstrangelooker dismissed rollyjoel’s stale review November 10, 2021 00:41

Found the bug and fixed it.

@github-actions
Copy link
Contributor

Typescript Tests

    7 files    76 suites   5m 8s ⏱️
167 tests 163 ✔️   4 💤 0 ❌
582 runs  566 ✔️ 16 💤 0 ❌

Results for commit 75298ac.

@jkaster jkaster self-requested a review November 10, 2021 01:01
Copy link

@jkaster jkaster left a comment

Choose a reason for hiding this comment

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

LGTM

@drstrangelooker drstrangelooker merged commit 304d0d0 into main Nov 10, 2021
@drstrangelooker drstrangelooker deleted the drstrangelove/release-21.20 branch November 10, 2021 01:04
@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants