Skip to content

Conversation

ArthurZucker
Copy link
Collaborator

@ArthurZucker ArthurZucker commented Sep 5, 2023

What does this PR do?

Fixes the main branch after the merge of #25895 which has a typo when checking the outputs of the ci tests.

Two part fix:

  1. Fatal errors were not shown in the outputs so we don't know what was wrong
- if x.startswith("FAILED ")]); fp.close(); fp = open("summary_short.txt", "w"); fp.write(failed); fp.close()'
+ if x.startswith("FAILED ", "ERROR ")]); fp.close(); fp = open("summary_short.txt", "w"); fp.write(failed); fp.close()'
  1. Fix the error:
- check_test_command = f'if [ -s reports/test_{self.name}/failures_short.txt ]; '
+ check_test_command = f'if [ -s reports/{self.name}/failures_short.txt ]; '

should do the trick

  1. There is now 8d51801 that broke main, marking the test as slow 😉

@ArthurZucker ArthurZucker requested a review from ydshieh September 5, 2023 17:29
Copy link
Contributor

@amyeroberts amyeroberts left a comment

Choose a reason for hiding this comment

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

Thanks for fixing!

steps.append({"run": {"name": "Run tests", "command": test_command}})

check_test_command = f'if [ -s reports/tests_{self.name}/failures_short.txt ]; '
check_test_command = f'if [ -s reports/{self.job_name}/failures_short.txt ]; '
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The test_ is added to self.name in self.job_name based on a rule. self.jon_name is the one that is used when these files are created.

Copy link
Member

@LysandreJik LysandreJik left a comment

Choose a reason for hiding this comment

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

LGTM

@HuggingFaceDocBuilderDev
Copy link

HuggingFaceDocBuilderDev commented Sep 5, 2023

The documentation is not available anymore as the PR was closed or merged.

@ArthurZucker ArthurZucker merged commit d0354e5 into huggingface:main Sep 5, 2023
@ArthurZucker ArthurZucker deleted the fix-main-ci branch September 5, 2023 18:16
self.assertEqual(output, {"text": "A MAN SAID TO THE UNIVERSE SIR I EXIST"})

@require_torch
@slow
Copy link
Contributor

@sanchit-gandhi sanchit-gandhi Sep 6, 2023

Choose a reason for hiding this comment

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

Looks like this test is not related to the commit that was merged yesterday, which didn't touch this test: 8d51801

This probably failed due to using the whisper-tiny model in the test taking abnormally long, since we've not seen this failure before

Went through all the ASR pipeline tests and they're all correctly marked as slow / not slow now 👍

@ydshieh ydshieh mentioned this pull request Sep 7, 2023
parambharat pushed a commit to parambharat/transformers that referenced this pull request Sep 26, 2023
* start with error too

* fix ?

* start with nit

* one more path

* use `job_name`

* mark pipeline test as slow
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.

5 participants