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

[Tests] no-restricted-paths: skip failing tests on windows #2472

Conversation

AdriAt360
Copy link
Contributor

@AdriAt360 AdriAt360 commented Jun 8, 2022

following #2466 (comment)

Context

One test is failing on windows, and multiple attempts to fix it have failed, without understanding why it's still failing

Changes

Skip this test, waiting for a bright idea

@AdriAt360
Copy link
Contributor Author

awesome, the test is broken here too, I can close
image

@AdriAt360 AdriAt360 closed this Jun 8, 2022
@ljharb
Copy link
Member

ljharb commented Jun 8, 2022

Please don’t open throwaway PRs like this tho; a PR ref is a permanent mark on a repo that can’t ever be removed.

@AdriAt360
Copy link
Contributor Author

@ljharb got it 👍🏽
what's the preferred way of investigations needing to run the pipelines with unrelated changes to an existing PR?

@ljharb
Copy link
Member

ljharb commented Jun 8, 2022

Ask me to push a branch, or set up appveyor on your own fork.

@AdriAt360 AdriAt360 changed the title ⚠️ do not merge here, investigation only [Tests] no-restricted-paths: skip failing tests on windows native Jun 21, 2022
@AdriAt360 AdriAt360 changed the title [Tests] no-restricted-paths: skip failing tests on windows native [Tests] no-restricted-paths: skip failing tests on windows Jun 21, 2022
@AdriAt360
Copy link
Contributor Author

@ljharb it seems I can't reopen here after force pushing a new commit, are you able to with your admin rights by any chance?

@AdriAt360
Copy link
Contributor Author

@ljharb note that I commented out the test on #2466 anyways, so we can just merge the other PR if it's ok for you, as it would be simpler
your call

@ljharb
Copy link
Member

ljharb commented Jun 21, 2022

No, if the branch was deleted the PR can never be reopened - it would have had to be reopened first.

Commenting out the test isn’t the proper way to skip it, and we have to land that skipping separately, before #2466.

@AdriAt360
Copy link
Contributor Author

@ljharb in practice, there was no existing failing test, it was only uncovered code

  • here I only added a failing test to double-check that the existing code was also faulty
  • so even without changing this code, there is no CI issue currently

Commenting out the test isn’t the proper way to skip it

what would be the correct way? I couldn't find any clue in the code
by the way, feel free to answer on #2466 directly as I commented out a test there to skip it

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants