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

refactor and remove tests/scripts/test_all_pip.py #7659

Closed
1 of 2 tasks
brainwane opened this issue Jan 27, 2020 · 3 comments · Fixed by #7680
Closed
1 of 2 tasks

refactor and remove tests/scripts/test_all_pip.py #7659

brainwane opened this issue Jan 27, 2020 · 3 comments · Fixed by #7680
Labels
auto-locked Outdated issues that have been locked by automation C: tests Testing and related things type: maintenance Related to Development and Maintenance Processes type: refactor Refactoring code

Comments

@brainwane
Copy link
Contributor

brainwane commented Jan 27, 2020

Per developer docs, the single file in tests/scripts/(test_all_pip.py):

will probably die in future in a refactor -- scripts for running all of the tests, but we use pytest now. Someone could make a PR to remove this! Good first issue!

Is test_all_pip.py actually used anywhere? I think it's not but I'm not sure, and reviewing and refactoring it looks too complicated to be a good first issue.

Looks like the necessary steps are:

  • verify that test_all_pip is not actually used anywhere in our testing or in our downstreams' tests
  • git rm the file and the scripts/ directory

Is that correct? cc @pradyunsg

@triage-new-issues triage-new-issues bot added the S: needs triage Issues/PRs that need to be triaged label Jan 27, 2020
@brainwane brainwane added C: tests Testing and related things type: maintenance Related to Development and Maintenance Processes type: refactor Refactoring code labels Jan 27, 2020
@triage-new-issues triage-new-issues bot removed the S: needs triage Issues/PRs that need to be triaged label Jan 27, 2020
@Bhavam
Copy link
Contributor

Bhavam commented Feb 2, 2020

Hi , so I checked out the code base and found no use of the script test_all_pip.py , I'll create a PR for version with the folder tests/scripts removed.
I hope that's agreeable if not please suggest any more changes required for the issue #7659 .

@pradyunsg
Copy link
Member

pradyunsg commented Feb 2, 2020

Please make sure you mention the PR number here, once you file the PR. Use the #nnnn format, which would result in links to-and-from the relevant issue. :)

@pradyunsg
Copy link
Member

verify that test_all_pip is not actually used anywhere in our testing

It is not; verified.

in our downstreams' tests

I don't think so, but it shouldn't matter much to us -- IMO if they're still using this, they should probably update to invoke via pytest; or they can maintain a patch to restore this file.

@lock lock bot added the auto-locked Outdated issues that have been locked by automation label Mar 10, 2020
@lock lock bot locked as resolved and limited conversation to collaborators Mar 10, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
auto-locked Outdated issues that have been locked by automation C: tests Testing and related things type: maintenance Related to Development and Maintenance Processes type: refactor Refactoring code
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants