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

to open expected outpout file with an encoding parameter #717

Merged

Conversation

iuhilnehc-ynos
Copy link
Contributor

I need to add a test in ros2/rcutils#426, but it does not work if a regex file contains some escape character. (e.g. "\b")

Chen Lihui added 2 commits July 12, 2023 07:49
Signed-off-by: Chen Lihui <lihui.chen@sony.com>
Signed-off-by: Chen Lihui <lihui.chen@sony.com>
@iuhilnehc-ynos iuhilnehc-ynos changed the title to open regex file with a encoding parameter to open expected outpout file with a encoding parameter Jul 12, 2023
@iuhilnehc-ynos iuhilnehc-ynos changed the title to open expected outpout file with a encoding parameter to open expected outpout file with an encoding parameter Jul 12, 2023
Chen Lihui added 2 commits July 12, 2023 12:48
Signed-off-by: Chen Lihui <lihui.chen@sony.com>
Signed-off-by: Chen Lihui <lihui.chen@sony.com>
Copy link

@fujitatomoya fujitatomoya left a comment

Choose a reason for hiding this comment

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

lgtm

@fujitatomoya
Copy link

@adityapande-1995 could we have another review?

@@ -64,7 +65,7 @@ def _filter(output):
return _filter


def expected_output_from_file(path):
def expected_output_from_file(path, encoding: Optional[str] = None):
Copy link
Contributor

Choose a reason for hiding this comment

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

While we are adding in new type annotations, can we add one to path as well?

Suggested change
def expected_output_from_file(path, encoding: Optional[str] = None):
def expected_output_from_file(path: str, encoding: Optional[str] = None):

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. Updated in e445d13

Chen Lihui and others added 2 commits August 25, 2023 08:37
Co-authored-by: Chris Lalancette <clalancette@gmail.com>
Signed-off-by: Chen Lihui <lihui.chen@sony.com>
Copy link
Contributor

@clalancette clalancette left a comment

Choose a reason for hiding this comment

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

Looks good to me with green CI.

@iuhilnehc-ynos
Copy link
Contributor Author

CI with repos:
build/test_args: --packages-above-and-dependencies launch_testing

  • Linux Build Status
  • Linux-aarch64 Build Status
  • Windows Build Status

@clalancette clalancette merged commit 6ee6660 into ros2:rolling Sep 13, 2023
2 checks passed
wjwwood pushed a commit that referenced this pull request Sep 20, 2023
* add an option to read the regex file by escape mode

Signed-off-by: Chen Lihui <lihui.chen@sony.com>
Co-authored-by: Chris Lalancette <clalancette@gmail.com>
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.

None yet

3 participants