◐ Shell
reader mode source ↗
Skip to content

Fix #4157: handle error in case of unsupported format#4474

Closed
itsankitkp wants to merge 47 commits into
RustPython:mainfrom
itsankitkp:handle-panic-strftime
Closed

Fix #4157: handle error in case of unsupported format#4474
itsankitkp wants to merge 47 commits into
RustPython:mainfrom
itsankitkp:handle-panic-strftime

Conversation

@itsankitkp

@itsankitkp itsankitkp commented Jan 27, 2023

Copy link
Copy Markdown
Contributor

Chrono panics in case of unsupported formats, this patch handles such cases and returns supplied format as a result.

@itsankitkp itsankitkp mentioned this pull request Jan 27, 2023
Closed
@itsankitkp itsankitkp changed the title handle error in case of unsupported format Jan 27, 2023
@itsankitkp itsankitkp requested a review from fanninpm January 27, 2023 19:47
@fanninpm

fanninpm commented Jan 27, 2023

Copy link
Copy Markdown
Contributor

It's fine to have that "%4Y" case in the extra_tests/.

@itsankitkp

itsankitkp commented Jan 27, 2023

Copy link
Copy Markdown
Contributor Author

It's fine to have that "%4Y" case in the extra_tests/.

Okay, I tried that but there is an issue
If I write assert_equal(_time.strftime("%4Y"), "%4Y"),
it runs first arg against cpython (which returns "2023") and 2nd part against rustpython (which returns "%4Y")
What I require is to run both first and second argument against rustpython only. Am I looking at wrong direction here?

Assertion Failure: <class 'str'>(2023) == <class 'str'>(%4Y)

@DimitrisJim DimitrisJim linked an issue Jan 30, 2023 that may be closed by this pull request
time.strftime(arg) fails when arg is not valid format string #4157
Closed
@itsankitkp itsankitkp force-pushed the handle-panic-strftime branch from 0cf0b04 to d4c2fd3 Compare January 31, 2023 15:30
@itsankitkp itsankitkp requested a review from fanninpm January 31, 2023 15:31
@itsankitkp

itsankitkp commented Jan 31, 2023

Copy link
Copy Markdown
Contributor Author

It's fine to have that "%4Y" case in the extra_tests/.

Okay, I tried that but there is an issue If I write assert_equal(_time.strftime("%4Y"), "%4Y"), it runs first arg against cpython (which returns "2023") and 2nd part against rustpython (which returns "%4Y") What I require is to run both first and second argument against rustpython only. Am I looking at wrong direction here?

Assertion Failure: <class 'str'>(2023) == <class 'str'>(%4Y)

I have fixed this for now by using unsupported format which returns same result in both python and rustpython.

@youknowone

youknowone commented Feb 17, 2023

Copy link
Copy Markdown
Member

@itsankitkp it passes tests on ubuntu, but not on macos and windows. could you check it?

----------------------------- Captured stdout call -----------------------------
Assertion Failure: <class 'str'>(?) == <class 'str'>(%?)
----------------------------- Captured stderr call -----------------------------
Traceback (most recent call last):
  File "/Users/runner/work/RustPython/RustPython/extra_tests/snippets/stdlib_datetime.py", line 135, in <module>
    assert_equal(_time.strftime("%?"), "%?")
  File "/Users/runner/work/RustPython/RustPython/extra_tests/snippets/testutils.py", line 58, in assert_equal
    _assert_print(lambda: a == b, [_typed(a), '==', _typed(b)])
  File "/Users/runner/work/RustPython/RustPython/extra_tests/snippets/testutils.py", line 47, in _assert_print
    assert f()
AssertionError

@itsankitkp itsankitkp force-pushed the handle-panic-strftime branch from fd2f370 to 052e984 Compare February 18, 2023 08:30
DimitrisJim and others added 17 commits February 19, 2023 11:06
@DimitrisJim @itsankitkp
Fix clippy issues for rust 1.67 (RustPython#4478)
eb5dd55
@discord9 @itsankitkp
feat: allow not set SIGINT handler
1d0cb29
@discord9 @itsankitkp
feat: impl Detail option for not set SIG_INT
844ff75
@nicku12345 @itsankitkp
use qualname in TypeErrors for functions (RustPython#4476)
3a08ff1
@DimitrisJim @itsankitkp
Match on ascii start/continuation characters before calling functions.
29e322b
@DimitrisJim @itsankitkp
Don't call is_emoji_presentation for each invocation of consume_normal
516c41d
@DimitrisJim @itsankitkp
Add initial capacities, use u32s for indents/spaces.
a3eb988
@DimitrisJim @itsankitkp
Eat for comma.
6ba8191
@DimitrisJim @itsankitkp
Hint that the unwrap should always succeed.
4c37307
@youknowone @itsankitkp
use workspace dependencies
2cb6634
@dependabot @itsankitkp
Bump openssl-src from 111.24.0+1.1.1s to 111.25.0+1.1.1t …
2ce349d 
Bumps [openssl-src](https://github.com/alexcrichton/openssl-src-rs) from 111.24.0+1.1.1s to 111.25.0+1.1.1t.
- [Release notes](https://github.com/alexcrichton/openssl-src-rs/releases)
- [Commits](https://github.com/alexcrichton/openssl-src-rs/commits)

---
updated-dependencies:
- dependency-name: openssl-src
  dependency-type: indirect
...

Signed-off-by: dependabot[bot] <support@github.com>
@DimitrisJim @itsankitkp
Move NewLineHandler inline, don't check each character twice.
01380bf
@DimitrisJim @itsankitkp
Document lexer.
8ef74d6
@youknowone @itsankitkp
Try to fix mac build
0d3ff4d
@youknowone @itsankitkp
update libffi
c682063
@youknowone @itsankitkp
bump up openssl and libffi
1f9a48f
@youknowone @itsankitkp
skip run rust tests for macOS CI
600a3da
39 hidden items Load more…
DimitrisJim and others added 23 commits February 19, 2023 11:06
@DimitrisJim @itsankitkp
Add tests, some comments, to function.rs.
e53e891
@youknowone @itsankitkp
extra_tests/snippets/{builtins => builtin_eval}.py
9fc54fd
@youknowone @itsankitkp
remove duplicated tests from tests/stdlib_math.py
a8d63ff
@youknowone @itsankitkp
Remove completed TODO
1b6d45d
@youknowone @itsankitkp
wrap_index without abs
6871964
@youknowone @itsankitkp
optimize str.(l|r)strip
6731150
@jonathanslenders @itsankitkp
Fix aarch64 compatibility for sqlite.
0c7324e
@youknowone @itsankitkp
Fix str.join with str subclass
c45bcfa
@minhrongcon2000 @itsankitkp
Add co_freevars to code object
96178a1
@minhrongcon2000 @itsankitkp
Fix code linting
9ac6e99
@howjmay @itsankitkp
Add co_cellvars to code object …
c5a629b 
Closes RustPython#4501
@DimitrisJim @itsankitkp
Fix unexpected success in test_future.
1755f04
@coolreader18 @itsankitkp
Use nix for more things
390c8da
@coolreader18 @itsankitkp
Tidy up ssl a little
8bac26f
@coolreader18 @itsankitkp
Run cargo-update
39ddc50
@youknowone @itsankitkp
PyObjectRef::downcast_exact returns PyRefExact
59c7536
@youknowone @itsankitkp
Add description for PyRefExact
c97d504
@xiaozhiyan @itsankitkp
Optimize Py<PyDict>::to_attributes() to reuse PyRefExact
ef5ba76
@dannasman @itsankitkp
Optimize bytes-like (l|r)strip (RustPython#4500)
b1f41c1
@youknowone @itsankitkp
narrow publicity of BytesInner::elements
900f49a
@dannasman @itsankitkp
remove unnecessary to_vec()
ad0b15e
@itsankitkp
Handle panic in case of unsupported format in chrono …
5b17bd6 
Chrono panics in case of unsupported formats, this patch handles such
cases and returns supplied format as a result.
@itsankitkp
feat! add test case for unsupported time format
d4a6d39
@itsankitkp itsankitkp force-pushed the handle-panic-strftime branch from be812d7 to d4a6d39 Compare February 19, 2023 05:37
@youknowone

youknowone commented Feb 20, 2023

Copy link
Copy Markdown
Member

rebase seems messed up. could you try:

git fetch upstream
git rebase upstream/main

when upstream is github.com/RustPython/RustPython

@itsankitkp

itsankitkp commented Feb 20, 2023

Copy link
Copy Markdown
Contributor Author

rebase seems messed up. could you try:

git fetch upstream
git rebase upstream/main

when upstream is github.com/RustPython/RustPython

This branch was messed up, I have created fresh PR with same changes with history fixed.
Can you please take a look #4530

@itsankitkp

itsankitkp commented Feb 20, 2023

Copy link
Copy Markdown
Contributor Author

Closing as I have opened other PR: #4530 with fixed git history

@itsankitkp itsankitkp closed this Feb 20, 2023
@fanninpm fanninpm mentioned this pull request Feb 23, 2023
Merged
5 tasks
@youknowone youknowone mentioned this pull request Feb 23, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@fanninpm fanninpm Awaiting requested review from fanninpm

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

time.strftime(arg) fails when arg is not valid format string