Skip to content

Restrict unicode sent to Github API #251

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

Merged
merged 3 commits into from
Apr 23, 2022

Conversation

EnricoMi
Copy link
Owner

@EnricoMi EnricoMi commented Apr 23, 2022

Fixes #250. Likely related to #212.

@github-actions

This comment has been minimized.

@github-actions
Copy link

github-actions bot commented Apr 23, 2022

Unit Test Results (Linux)

       54 files  ±    0         54 suites  ±0   2m 38s ⏱️ +7s
     271 tests +    4       271 ✔️ +    4      0 💤 ±0  0 ±0 
14 634 runs  +216  14 202 ✔️ +216  432 💤 ±0  0 ±0 

Results for commit 4f8694b. ± Comparison against base commit 0ee9ecd.

♻️ This comment has been updated with latest results.

@github-actions
Copy link

github-actions bot commented Apr 23, 2022

Unit Test Results (macOS)

       54 files  ±    0         54 suites  ±0   2m 38s ⏱️ +7s
     271 tests +    4       271 ✔️ +    4      0 💤 ±0  0 ±0 
14 634 runs  +216  14 202 ✔️ +216  432 💤 ±0  0 ±0 

Results for commit 4f8694b. ± Comparison against base commit 0ee9ecd.

♻️ This comment has been updated with latest results.

@github-actions
Copy link

github-actions bot commented Apr 23, 2022

Unit Test Results (Dockerfile)

       54 files  ±    0         54 suites  ±0   2m 38s ⏱️ +7s
     271 tests +    4       271 ✔️ +    4      0 💤 ±0  0 ±0 
14 634 runs  +216  14 202 ✔️ +216  432 💤 ±0  0 ±0 

Results for commit 4f8694b. ± Comparison against base commit 0ee9ecd.

♻️ This comment has been updated with latest results.

@github-actions
Copy link

github-actions bot commented Apr 23, 2022

Unit Test Results (Docker Image)

       54 files  ±    0         54 suites  ±0   2m 38s ⏱️ +7s
     271 tests +    4       271 ✔️ +    4      0 💤 ±0  0 ±0 
14 634 runs  +216  14 202 ✔️ +216  432 💤 ±0  0 ±0 

Results for commit 4f8694b. ± Comparison against base commit 0ee9ecd.

♻️ This comment has been updated with latest results.

@github-actions
Copy link

github-actions bot commented Apr 23, 2022

Unit Test Results (Windows)

       54 files  ±    0         54 suites  ±0   2m 38s ⏱️ +7s
     271 tests +    4       271 ✔️ +    4      0 💤 ±0  0 ±0 
14 634 runs  +216  14 202 ✔️ +216  432 💤 ±0  0 ±0 

Results for commit 4f8694b. ± Comparison against base commit 0ee9ecd.

♻️ This comment has been updated with latest results.

@github-actions
Copy link

github-actions bot commented Apr 23, 2022

Unit Test Results (reference)

       54 files  ±    0         54 suites  ±0   2m 38s ⏱️ +7s
     271 tests +    4       271 ✔️ +    4      0 💤 ±0  0 ±0 
14 634 runs  +216  14 202 ✔️ +216  432 💤 ±0  0 ±0 

Results for commit 4f8694b. ± Comparison against base commit 0ee9ecd.

♻️ This comment has been updated with latest results.

@EnricoMi EnricoMi force-pushed the branch-restrict-json-payload-unicode branch from 027c250 to ef4f2b3 Compare April 23, 2022 20:36
@github-actions

This comment has been minimized.



def restrict_unicode_list(texts: List[str]) -> List[str]:
return [restrict_unicode(text) for text in texts]
Copy link

Choose a reason for hiding this comment

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

Incompatible return type: Expected List[str] but got List[Optional[str]].

(at-me in a reply with help or ignore)


Was this a good recommendation?
[ 🙁 Not relevant ] - [ 😕 Won't fix ] - [ 😑 Not critical, will fix ] - [ 🙂 Critical, will fix ] - [ 😊 Critical, fixing now ]

@github-actions
Copy link

github-actions bot commented Apr 23, 2022

Unit Test Results (Test Files)

  26 files  +1      4 errors  23 suites  +1   39m 21s ⏱️ +9s
279 tests +7  228 ✔️ +1  20 💤 +2  25 +2  6 🔥 +2 
450 runs  +7  358 ✔️ +1  57 💤 +2  28 +2  7 🔥 +2 

For more details on these parsing errors, failures and errors, see this check.

Results for commit 4f8694b. ± Comparison against base commit 0ee9ecd.

♻️ This comment has been updated with latest results.

@EnricoMi EnricoMi marked this pull request as ready for review April 23, 2022 20:44


def get_skipped_tests_list_annotation(cases: UnitTestCaseResults, max_chunk_size: int = 64000) -> List[Annotation]:
return get_test_list_annotation(get_skipped_tests_list(cases), 'skipped test', max_chunk_size)
return get_test_list_annotation(restrict_unicode_list(get_skipped_tests_list(cases)), 'skipped test', max_chunk_size)
Copy link

Choose a reason for hiding this comment

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

Incompatible parameter type: Expected List[str] for 1st positional only parameter to call get_test_list_annotation but got List[Optional[str]].

(at-me in a reply with help or ignore)


Was this a good recommendation?
[ 🙁 Not relevant ] - [ 😕 Won't fix ] - [ 😑 Not critical, will fix ] - [ 🙂 Critical, will fix ] - [ 😊 Critical, fixing now ]

@@ -758,11 +771,11 @@ def get_skipped_tests_list(cases: UnitTestCaseResults) -> List[str]:


def get_all_tests_list_annotation(cases: UnitTestCaseResults, max_chunk_size: int = 64000) -> List[Annotation]:
return get_test_list_annotation(get_all_tests_list(cases), 'test', max_chunk_size)
return get_test_list_annotation(restrict_unicode_list(get_all_tests_list(cases)), 'test', max_chunk_size)
Copy link

Choose a reason for hiding this comment

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

Incompatible parameter type: Expected List[str] for 1st positional only parameter to call get_test_list_annotation but got List[Optional[str]].

(at-me in a reply with help or ignore)


Was this a good recommendation?
[ 🙁 Not relevant ] - [ 😕 Won't fix ] - [ 😑 Not critical, will fix ] - [ 🙂 Critical, will fix ] - [ 😊 Critical, fixing now ]



def get_skipped_tests_list_annotation(cases: UnitTestCaseResults, max_chunk_size: int = 64000) -> List[Annotation]:
return get_test_list_annotation(get_skipped_tests_list(cases), 'skipped test', max_chunk_size)
return get_test_list_annotation(restrict_unicode_list(get_skipped_tests_list(cases)), 'skipped test', max_chunk_size)
Copy link

Choose a reason for hiding this comment

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

Incompatible parameter type: Expected List[Optional[str]] for 1st positional only parameter to call restrict_unicode_list but got List[str].

(at-me in a reply with help or ignore)


Was this a good recommendation?
[ 🙁 Not relevant ] - [ 😕 Won't fix ] - [ 😑 Not critical, will fix ] - [ 🙂 Critical, will fix ] - [ 😊 Critical, fixing now ]

@@ -758,11 +771,11 @@ def get_skipped_tests_list(cases: UnitTestCaseResults) -> List[str]:


def get_all_tests_list_annotation(cases: UnitTestCaseResults, max_chunk_size: int = 64000) -> List[Annotation]:
return get_test_list_annotation(get_all_tests_list(cases), 'test', max_chunk_size)
return get_test_list_annotation(restrict_unicode_list(get_all_tests_list(cases)), 'test', max_chunk_size)
Copy link

Choose a reason for hiding this comment

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

Incompatible parameter type: Expected List[Optional[str]] for 1st positional only parameter to call restrict_unicode_list but got List[str].

(at-me in a reply with help or ignore)


Was this a good recommendation?
[ 🙁 Not relevant ] - [ 😕 Won't fix ] - [ 😑 Not critical, will fix ] - [ 🙂 Critical, will fix ] - [ 😊 Critical, fixing now ]

@github-actions
Copy link

Unit Test Results (setup-python)

       54 files  ±    0         54 suites  ±0   2m 38s ⏱️ +7s
     271 tests +    4       271 ✔️ +    4      0 💤 ±0  0 ±0 
14 634 runs  +216  14 202 ✔️ +216  432 💤 ±0  0 ±0 

Results for commit 4f8694b. ± Comparison against base commit 0ee9ecd.

@EnricoMi EnricoMi merged commit 1c80588 into master Apr 23, 2022
@EnricoMi EnricoMi deleted the branch-restrict-json-payload-unicode branch April 23, 2022 21:11
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 this pull request may close these issues.

Action fails with Validation Failed
1 participant