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

Fix CI slack reporting issue #34833

Merged
merged 5 commits into from
Nov 20, 2024
Merged

Fix CI slack reporting issue #34833

merged 5 commits into from
Nov 20, 2024

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
25 checks passed
@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>
BernardZach pushed a commit to innovationcore/transformers that referenced this pull request Dec 6, 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.

3 participants
  NODES
COMMUNITY 2
innovation 1
Project 5
USERS 3