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

pythonPackages.chevron: init at 0.13.1 #59088

Merged
merged 2 commits into from Apr 8, 2019

Conversation

dhl
Copy link
Contributor

@dhl dhl commented Apr 7, 2019

A Python implementation of mustache

This is being packaged as part of the effort to update aws-sam-cli to the latest version

Motivation for this change
Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nix-review --run "nix-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Assured whether relevant documentation is up to date
  • Fits CONTRIBUTING.md.

Copy link
Contributor

@worldofpeace worldofpeace left a comment

Choose a reason for hiding this comment

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

Just the usual packaging nitpicks.

Mainly tests need to be enabled somehow, see comments below.

pkgs/development/python-modules/chevron/default.nix Outdated Show resolved Hide resolved
pkgs/development/python-modules/chevron/default.nix Outdated Show resolved Hide resolved
pkgs/development/python-modules/chevron/default.nix Outdated Show resolved Hide resolved
pkgs/development/python-modules/chevron/default.nix Outdated Show resolved Hide resolved
@worldofpeace
Copy link
Contributor

I'd like to merge this before #59089 and you've added yourself to the maintainers list in both.

Can you have one commit for doing that here?

like:

maintainers: add dhkl

@dhl
Copy link
Contributor Author

dhl commented Apr 7, 2019

@worldofpeace thanks for taking the time to review this. Will get the changes in ASAP.

@dhl dhl force-pushed the dhl/python-modules/chevron branch from cd68875 to 8185469 Compare April 7, 2019 13:12
@dhl
Copy link
Contributor Author

dhl commented Apr 7, 2019

@worldofpeace I've split the commit into a maintainers and a package commit.

@worldofpeace
Copy link
Contributor

I actually didn't see the tests being ran in the output.

buildPython* does the following for packages that use setuptools

${python.pythonForBuild.interpreter} nix_run_setup test

Looks like we actually just need to use a particular file

diff --git a/pkgs/development/python-modules/chevron/default.nix b/pkgs/development/python-modules/chevron/default.nix
index 39fb9d464d0..04de410df68 100644
--- a/pkgs/development/python-modules/chevron/default.nix
+++ b/pkgs/development/python-modules/chevron/default.nix
@@ -1,6 +1,7 @@
 { lib
 , buildPythonPackage
 , fetchFromGitHub
+, python
 }:
 
 buildPythonPackage rec {
@@ -15,6 +16,10 @@ buildPythonPackage rec {
     sha256 = "0l1ik8dvi6bgyb3ym0w4ii9dh25nzy0x4yawf4zbcyvvcb6af470";
   };
 
+  checkPhase = ''
+    ${python.interpreter} test_spec.py
+  '';
+
     meta = with lib; {
     homepage = https://github.com/noahmorrison/chevron;
     description = "A python implementation of the mustache templating language";

@worldofpeace worldofpeace mentioned this pull request Apr 8, 2019
10 tasks
@dhl
Copy link
Contributor Author

dhl commented Apr 8, 2019

The tests did run for me as is:

running install tests
running test
running egg_info
writing chevron.egg-info/PKG-INFO
writing top-level names to chevron.egg-info/top_level.txt
writing dependency_links to chevron.egg-info/dependency_links.txt
writing entry points to chevron.egg-info/entry_points.txt
reading manifest file 'chevron.egg-info/SOURCES.txt'
writing manifest file 'chevron.egg-info/SOURCES.txt'
running build_ext
test_bad_set_delimiter_tag (test_spec.ExpandedCoverage) ... ok
test_callable_1 (test_spec.ExpandedCoverage) ... ok
test_callable_2 (test_spec.ExpandedCoverage) ... ok
test_callable_3 (test_spec.ExpandedCoverage)
Test generating some data within the function ... ok
test_closing_tag_only (test_spec.ExpandedCoverage) ... ok
test_complex (test_spec.ExpandedCoverage) ... ok
test_current_line_rest (test_spec.ExpandedCoverage) ... ok
test_falsy (test_spec.ExpandedCoverage) ... ok
test_inverted_coercion (test_spec.ExpandedCoverage) ... ok
test_listed_data (test_spec.ExpandedCoverage) ... ok
test_main (test_spec.ExpandedCoverage) ... ok
test_nest_loops_with_same_key (test_spec.ExpandedCoverage) ... ok
test_no_opening_tag (test_spec.ExpandedCoverage) ... ok
test_recursion (test_spec.ExpandedCoverage) ... ok
test_unclosed_sections (test_spec.ExpandedCoverage) ... ok
test_unicode_basic (test_spec.ExpandedCoverage) ... ok
test_unicode_inside_list (test_spec.ExpandedCoverage) ... ok
test_unicode_partial (test_spec.ExpandedCoverage) ... ok
test_unicode_variable (test_spec.ExpandedCoverage) ... ok

----------------------------------------------------------------------
Ran 19 tests in 0.005s

OK

@worldofpeace
Copy link
Contributor

They succeed for me for python 2.7 but for any other version they failed with https://gist.github.com/worldofpeace/f307facbcb2e453d09e69112bafc2517 if didn't use the previous patch

A Python implementation of mustache
@dhl dhl force-pushed the dhl/python-modules/chevron branch from 8185469 to 7033cf0 Compare April 8, 2019 05:19
@dhl
Copy link
Contributor Author

dhl commented Apr 8, 2019

Added the checkPhase fix. Thanks @worldofpeace!

Copy link
Contributor

@worldofpeace worldofpeace left a comment

Choose a reason for hiding this comment

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

Built every package for every interpreter version.

@worldofpeace worldofpeace merged commit 2e7cb73 into NixOS:master Apr 8, 2019
@dhl dhl deleted the dhl/python-modules/chevron branch April 8, 2019 15:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants