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

Fail tests on flake8 failure #63

Merged
merged 4 commits into from Jun 21, 2021
Merged

Fail tests on flake8 failure #63

merged 4 commits into from Jun 21, 2021

Conversation

mohithg
Copy link
Contributor

@mohithg mohithg commented Jun 20, 2021

For syntax errors we anyways fail the test, for the other errors we show them as warnings, but if the total count of these exceeds 10 then we throw an error asking user to fix those lint errors

@mohithg mohithg changed the title Mohithg/fail tests Fail tests on flake8 failure Jun 20, 2021
@mohithg mohithg self-assigned this Jun 20, 2021
@mohithg mohithg linked an issue Jun 20, 2021 that may be closed by this pull request
@mohithg mohithg requested a review from guysmoilov June 20, 2021 09:55
@mohithg mohithg force-pushed the mohithg/fail-tests branch 17 times, most recently from cde3af2 to e056b6e Compare June 20, 2021 11:33
@mohithg mohithg marked this pull request as ready for review June 20, 2021 11:36
@guysmoilov
Copy link
Member

Why only if greater than 10 BTW?

@mohithg
Copy link
Contributor Author

mohithg commented Jun 20, 2021

@guysmoilov just some number because you asked in the ticket to have a certain range, so I thought 10 would be a good start. Its the sum by the way. So if the total weight of issues is 10 then it will fail. Should I reduce it to 5 ?

@guysmoilov
Copy link
Member

I guess I don't understand how flake8 works, I assumed we can set a "maximum allowed severity level"

@mohithg
Copy link
Contributor Author

mohithg commented Jun 20, 2021

Then we can do selective error codes like you did for other flake8 test you are already running

@mohithg
Copy link
Contributor Author

mohithg commented Jun 20, 2021

But I have slight preference over testing everything, and not just selected ones. Code is more hygiene that way.

@guysmoilov guysmoilov merged commit c134b8e into main Jun 21, 2021
@guysmoilov guysmoilov added the tests Tasks relating to tests label Jun 21, 2021
@mohithg mohithg deleted the mohithg/fail-tests branch October 30, 2021 12:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
tests Tasks relating to tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Fail tests on flake8 failure
2 participants