Skip to content

add bare environment CI tests #790

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.

Sign up for GitHub

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

Merged
merged 6 commits into from
Oct 21, 2019
Merged

add bare environment CI tests #790

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
d06f366
add bare linux test
wholmgren Oct 16, 2019
3f30845
fix test directory
wholmgren Oct 16, 2019
9dc049b
whatsnew
wholmgren Oct 16, 2019
ff445bc
moar pytest
wholmgren Oct 16, 2019
40c920b
what merge conflict?
wholmgren Oct 16, 2019
60b0e20
Merge remote-tracking branch 'pvlib/master' into cimin
wholmgren Oct 19, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 37 additions & 0 deletions azure-pipelines.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,43 @@ trigger:

jobs:

- job: 'Test_bare_linux'

pool:
vmImage: 'ubuntu-16.04'
strategy:
matrix:
Python35:
python.version: '3.5'
Python36:
python.version: '3.6'
Python37:
python.version: '3.7'

steps:
- task: UsePythonVersion@0
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just for my own information, where does the choice of @ versions come from here and below?

Copy link
Member Author

@wholmgren wholmgren Oct 17, 2019 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure that I understand the scope of the question, so let me know if this doesn't actually answer it. It's probably fine to just choose 1 python version for this bare environment test, but it's easy enough to add the matrix option so I figured why not. In general, we want to test pvlib against all supported Python versions. This is stated in the readme and installation docs as "3.5 and above". 3.8 just came out, so I will look into adding that in a few weeks. I'm pretty confident that pvlib itself will not have any issues with 3.8, but it often takes a few weeks for the dependencies to release new wheels and/or Anaconda to add compiled packages.

Azure Pipelines does not yet provide a 3.8 environment, so it's definitely not yet an option on the bare environment test. Looks like it's not yet available from Anaconda defaults either.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

More specifically, how do you know what the @0, @1, and @3 suffixes refer to?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, I don’t understand that part of the spec. I’m just following the docs here https://docs.microsoft.com/en-us/azure/devops/pipelines/ecosystems/python?view=azure-devops

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Specifies the task version number: https://docs.microsoft.com/en-us/azure/devops/pipelines/process/tasks?view=azure-devops&tabs=yaml#task-versions

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh I think I get it now. Microsoft supplies those task versions so they don’t break people’s pipelines when they make changes.

inputs:
versionSpec: '$(python.version)'

- script: |
pip install pytest pytest-cov pytest-mock pytest-timeout pytest-azurepipelines
pip install -e .
pytest pvlib --junitxml=junit/test-results.xml --cov=com --cov-report=xml --cov-report=html
displayName: 'Test with pytest'

- task: PublishTestResults@2
condition: succeededOrFailed()
inputs:
testResultsFiles: '**/test-*.xml'
testRunTitle: 'Publish test results for Python $(python.version)'

- task: PublishCodeCoverageResults@1
inputs:
codeCoverageTool: Cobertura
summaryFileLocation: '$(System.DefaultWorkingDirectory)/**/coverage.xml'
reportDirectory: '$(System.DefaultWorkingDirectory)/**/htmlcov'


- job: 'Test_conda_linux'

pool:
Expand Down
2 changes: 2 additions & 0 deletions docs/sphinx/source/whatsnew/v0.7.0.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,8 @@ Testing
in NSRDB (:issue:`733`)
* Added tests for methods in bifacial.py.
* Added tests for changes to cell temperature models.
* Add tests configuration for bare python environment (no conda).
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can't comment on the test configuration being added. But here: past tense "Added" or present tense "Add"? No reason to hold up the PR.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's a mix of past and present throughout the file, so I'm going to keep it as is for now. I'm ok setting a loose standard if someone wants to do the background research into what the rest of the python ecosystem uses. I can see arguments for either past or present. Separate issue.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Python release notes mix past and present tense, as does numpy. Not worth the trouble to create our own convention and enforce it.

(:issue:`727`)
* Added tests for changes to IAM models.
* Added test for `ModelChain.infer_aoi_model`.

Expand Down