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

Add ability to retest #734

Closed
wants to merge 2 commits into from

Conversation

OMEGARAZER
Copy link
Contributor

***Ability to retest won't be available till on the main branch

Adds ability to re-run tests if required.

Ignores if only markdown files are changed as they are handled by tox and weren't touched by these tests.

Adds basic Refurb settings to pyproject. Not adding as dev requirement as it's a nice to run but by no means needed.

Adds ability to re-run tests if required.

Ignores if only markdown files are changed as they are handled by tox and weren't touched by these tests.

Adds basic Refurb settings to pyproject. Not adding as dev requirement as it's a nice to run but by no means needed.

Updates black and isort versions.
@Serene-Arc
Copy link
Owner

What do you mean rerun tests?

@Serene-Arc
Copy link
Owner

Also please separate out the refurb stuff to another PR, we can add it to the pre-commit stuff as well, and move towards integrating it into the GH checks.

@OMEGARAZER
Copy link
Contributor Author

OMEGARAZER commented Dec 27, 2022

What do you mean rerun tests?

Like it says on the tin, lets the tests be re-run in situations like this where the tests are failing outside our control but need to pass before merging. Rather than being forced to make another change and hope the tests pass as they should, a new comment on the PR can be made with >retest-bdfr which should re-run the tests without needing to make any changes.

Also please separate out the refurb stuff to another PR, we can add it to the pre-commit stuff as well, and move towards integrating it into the GH checks.

I figured it wasn't worth another PR for 4 lines but if it's going to be integrated more I can move it out.

@Serene-Arc
Copy link
Owner

The thing is, I don't like this approach. I want to tests to be fixed rather than having this fix in place and complicating the testing process. If there's no way to fix them, then maybe this can be merged but there should be at least an attempt to fix the failing tests so they do so regularly or pass regularly.

@OMEGARAZER
Copy link
Contributor Author

I want to tests to be fixed rather than having this fix in place and complicating the testing process.

This doesn't change or alter any tests so I wouldn't call this a fix by any means and it never intended to be.

If there's no way to fix them

My point with this though is that in the situation like the one I pointed out there was nothing to fix.

Things like all the tests passing in my fork when I publish a change but then a test (seems mac mostly) will fail with 5xx/4xx errors in the PR on completely untouched sections.

Was just an attempt to lessen situations where tests are known good but failing outside of BDFR.

@OMEGARAZER OMEGARAZER closed this Dec 28, 2022
@Serene-Arc
Copy link
Owner

You don't have to close it, I just want to double check for reasons why it's failing on MacOS and see if there's any reasons for the failures first.

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.

2 participants