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

Fixes for virtualenv_version fact when virtualenv > 20.x #537

Merged
merged 1 commit into from
Apr 30, 2020
Merged

Fixes for virtualenv_version fact when virtualenv > 20.x #537

merged 1 commit into from
Apr 30, 2020

Conversation

pjonesIDBS
Copy link
Contributor

Pull Request (PR) description

This Pull Request (PR) fixes the following issues

@saz
Copy link
Contributor

saz commented Mar 25, 2020

Looks good to me.

Failing tests seems to be related to setuptools and deprecation of python 2

@bastelfreak
Copy link
Member

@pjonesIDBS I rebased your PR and it should be green now. However, can you add a test for it?

@bastelfreak bastelfreak added bug Something isn't working needs-tests labels Apr 26, 2020
@pjonesIDBS
Copy link
Contributor Author

Hi Tim (@bastelfreak),

Just cloned my fork of the module and inspected it in VSC. It looks like there's already accetance tests in spec/acceptance/facts_test_spec.rb. There's also a unit test in spec/unit/facter/virtualenv_version_spec.rb.

I converted the module locally to use PDK (which worked like a charm).

The easiest solution to test was to update the unit test for virtualenv_version.

I set the factor "virtualenv_version_output" to be what we see with a version > 20.x (in this case 20.0.17) and then validated the facter would equal 20.0.17 with the rest of the string removed.

All looks good. The builds are still passing as they did previously.

@saz
Copy link
Contributor

saz commented Apr 28, 2020

@pjonesIDBS I think it makes sense to check for both kinds of version, as this module might still be used with older virtualenv versions

Verified

This commit was signed with the committer’s verified signature. The key has expired.
bastelfreak Tim Meusel
This should fix issue #534 I've tested locally with success. I've tested against Virtualenv 20.0.4 and 16.7.9.
The output from the fact is only the version number as was previously the case.
@bastelfreak bastelfreak merged commit d91f26b into voxpupuli:master Apr 30, 2020
@pjonesIDBS pjonesIDBS deleted the patch-1 branch May 1, 2020 11:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants