-
Notifications
You must be signed in to change notification settings - Fork 19
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 processing selectors and tests. #75
Add processing selectors and tests. #75
Conversation
tests/test_jinja.py
Outdated
def test_jinja_x86(monkeypatch): | ||
template = "{{ x86 }}" | ||
|
||
platform_bkp = platform |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I need help here to mock this properly. I need to enforce platform.machine()
returns a given value (e.g., 'x86'
)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can use monkeypatch.setattr(platform, "machine", lambda: 'x86')
and let pytest revert it back for yoy; it is fine to call monkeypatch.setattr
multiple times too.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmmm I was almost there, but doing monkeypatch.setattr(platform, "machine", 'x86')
instead, which caused problems later.
Hi @allanleal, Thanks for the PR!
Usually we see the CI status here on the PR, but because your last commit in the branch had the string https://travis-ci.org/ESSS/conda-devenv/builds/492745664?utm_source=github_status&utm_medium=notification
Small GitHub tip: if write |
tests/test_jinja.py
Outdated
def test_jinja_x86(monkeypatch): | ||
template = "{{ x86 }}" | ||
|
||
platform_bkp = platform |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can use monkeypatch.setattr(platform, "machine", lambda: 'x86')
and let pytest revert it back for yoy; it is fine to call monkeypatch.setattr
multiple times too.
linting is failing due to an unused variable:
|
Tests are passing! 🎉 All is left is updating the docs. 🤗 |
:) I'll update the docs! |
Thanks a lot @allanleal! 🙇 |
conda_devenv/devenv.py
Outdated
"linux": islinux, | ||
"linux32": islinux and is32bit, | ||
"linux64": islinux and is64bit, | ||
"armv6l": None, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we want to let those with None
? I see you didn't add them to the docs.
Could you remove them? And maybe create a issue do add them later?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll see if I add them still in this PR. Before merging, I'll make a last call for approval just to be sure.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I ended up deleting those three variables with None and entering a new Issue - #77 .
docs/conf.py
Outdated
@@ -97,7 +97,7 @@ | |||
#show_authors = False | |||
|
|||
# The name of the Pygments (syntax highlighting) style to use. | |||
pygments_style = 'sphinx' | |||
pygments_style = 'default' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I changed this for local testing purposes and it remained here. Anyway, I can revert it back, but this one looks better on readthedocs.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We try to use the same theme and styles on all open source projects from ESSS.
I will ask to revert this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure, changed to sphinx
.
Okay -- all tests passed but one in travis (for py27). The error is a bit strange:
Forcing a new build with an amend commit. |
…hub.com:allanleal/conda-devenv into add-conda-build-preprocessing-selectors-support
docs/conf.py
Outdated
@@ -97,7 +97,7 @@ | |||
#show_authors = False | |||
|
|||
# The name of the Pygments (syntax highlighting) style to use. | |||
pygments_style = 'sphinx' | |||
pygments_style = 'default' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We try to use the same theme and styles on all open source projects from ESSS.
I will ask to revert this.
docs/conf.py
Outdated
@@ -111,7 +111,7 @@ | |||
|
|||
# The theme to use for HTML and HTML Help pages. See the documentation for | |||
# a list of builtin themes. | |||
html_theme = 'default' | |||
html_theme = 'sphinx' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ops...it's not here, but on pygments_style
.
Great - thanks @prusse-martin . |
Thank you for the work. |
That's OK by me - no worries! |
Need your help to check if tests are passing or failing (or if they are adequate).
I'll update the docs shortly.
This implements the features discussed in #74 .