Skip to content

Conversation

@ydshieh
Copy link
Collaborator

@ydshieh ydshieh commented Nov 20, 2024

What does this PR do?

#34812 causes the slack report fail to be sent.

This PR fixes this issue (a bit hacky)

@ydshieh ydshieh requested a review from ArthurZucker November 20, 2024 17:43
Comment on lines +2405 to +2420
lines = exception_message.split("\n")
# Add a first line with more informative information instead of just `= test session starts =`.
# This makes the `short test summary info` section more useful.
if "= test session starts =" in lines[0]:
text = ""
for line in lines[1:]:
if line.startswith("FAILED "):
text = line[len("FAILED ") :]
text = "".join(text.split(" - ")[1:])
elif line.startswith("=") and line.endswith("=") and " failed in " in line:
break
elif len(text) > 0:
text += f"\n{line}"
text = "(subprocess) " + text
lines = [text] + lines
exception_message = "\n".join(lines)
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

currently, what is shown in the (final) short test summary info is something like

/transformers/src/transformers/testing_utils.py:2420: ==== test session starts ====

which is not very informative.

And for the same test, we get two FAILED in the log: one from the subprocess and another one is the original pytest process. This causes slack CI report script fails.

Copy link
Collaborator

Choose a reason for hiding this comment

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

ah got it thanks for explaining

@HuggingFaceDocBuilderDev

The docs for this PR live here. All of your documentation changes will be reflected on that endpoint. The docs are available until 30 days after the last update.

Copy link
Collaborator

@ArthurZucker ArthurZucker left a comment

Choose a reason for hiding this comment

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

TY!

Comment on lines +2405 to +2420
lines = exception_message.split("\n")
# Add a first line with more informative information instead of just `= test session starts =`.
# This makes the `short test summary info` section more useful.
if "= test session starts =" in lines[0]:
text = ""
for line in lines[1:]:
if line.startswith("FAILED "):
text = line[len("FAILED ") :]
text = "".join(text.split(" - ")[1:])
elif line.startswith("=") and line.endswith("=") and " failed in " in line:
break
elif len(text) > 0:
text += f"\n{line}"
text = "(subprocess) " + text
lines = [text] + lines
exception_message = "\n".join(lines)
Copy link
Collaborator

Choose a reason for hiding this comment

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

ah got it thanks for explaining

@ydshieh ydshieh merged commit 40821a2 into main Nov 20, 2024
@ydshieh ydshieh deleted the check_report_2 branch November 20, 2024 20:36
BernardZach pushed a commit to BernardZach/transformers that referenced this pull request Dec 5, 2024
* fix

* fix

* fix

* fix

* fix

---------

Co-authored-by: ydshieh <ydshieh@users.noreply.github.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.

4 participants