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.

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

Jump to bottom

micawber: init at 0.3.5 #34948

Merged
merged 1 commit into from Feb 18, 2018
Merged

micawber: init at 0.3.5 #34948

merged 1 commit into from Feb 18, 2018

Conversation

davidak
Copy link
Member

@davidak davidak commented Feb 14, 2018 •
edited

Motivation for this change

Nikola needs it for the media shortcode. #33825

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option build-use-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 nox --run "nox-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Fits CONTRIBUTING.md.

@davidak davidak requested a review from FRidh as a code owner February 14, 2018 00:08
@davidak
Copy link
Member Author

davidak commented Feb 14, 2018

The test don't work. Can someone give me a hint how to get it working?

checking for references to /tmp/nix-build-python3.6-micawber-0.3.4.drv-0 in /nix/store/66p8373pfr4p2spr6y0kc39y88wvw8zf-python3.6-micawber-0.3.4...
running install tests
running test
running egg_info
writing micawber.egg-info/PKG-INFO
writing dependency_links to micawber.egg-info/dependency_links.txt
writing top-level names to micawber.egg-info/top_level.txt
reading manifest file 'micawber.egg-info/SOURCES.txt'
reading manifest template 'MANIFEST.in'
writing manifest file 'micawber.egg-info/SOURCES.txt'
running build_ext
runtests (unittest.loader._FailedTest) ... ERROR

======================================================================
ERROR: runtests (unittest.loader._FailedTest)
----------------------------------------------------------------------
ImportError: Failed to import test module: runtests
Traceback (most recent call last):
  File "/nix/store/87wc186a9xq5y6yxvar6f2sm5cbqzpzr-python3-3.6.4/lib/python3.6/unittest/loader.py", line 153, in loadTestsFromName
    module = __import__(module_name)
ModuleNotFoundError: No module named 'runtests'


----------------------------------------------------------------------
Ran 1 test in 0.000s

FAILED (errors=1)
Test failed: <unittest.runner.TextTestResult run=1 errors=1 failures=0>
error: Test failed: <unittest.runner.TextTestResult run=1 errors=1 failures=0>
builder for ‘/nix/store/vigdmz21p511z4jfwzk6wa2r3avl1pc5-python3.6-micawber-0.3.4.drv’ failed with exit code 1
error: build of ‘/nix/store/vigdmz21p511z4jfwzk6wa2r3avl1pc5-python3.6-micawber-0.3.4.drv’ failed

It may just need one dependency, but i don't know.

https://github.com/coleifer/micawber/blob/master/setup.py#L40

@davidak davidak changed the title add micawber at 0.3.4 micawber: init at 0.3.4 Feb 14, 2018
@disassembler
Copy link
Member

Try setting checkPhase = "${python.interpreter} runtests.py"; and see if tests run successfully. You'll probably need to add some dependencies to buildInputs.

@davidak
Copy link
Member Author

davidak commented Feb 14, 2018

The runtests.py is not in MANIFEST.in, therefore it is also not in the source bundle on PyPI.

Should we get it then from Github?

@davidak
Copy link
Member Author

davidak commented Feb 14, 2018

I fetch it now from Github and added BeautifulSoup. Now tests work.

@davidak davidak mentioned this pull request Feb 14, 2018
Closed
@dotlambda
Copy link
Member

@GrahamcOfBorg build python2.pkgs.micawber python3.pkgs.micawber

@GrahamcOfBorg
Copy link

Success on aarch64-linux (full log)

Partial log (click to expand)


----------------------------------------------------------------------
Ran 15 tests in 0.592s

OK
Running micawber tests
All micawber tests passed
Skipping django tests
/nix/store/1bdzmzmpsdhmmpxlv5a4zw092698lqxn-python2.7-micawber-0.3.4
/nix/store/75bxnvb197qgqvsydlxkpc5rgqwip1is-python3.6-micawber-0.3.4

@GrahamcOfBorg
Copy link

Success on x86_64-linux (full log)

Partial log (click to expand)


----------------------------------------------------------------------
Ran 15 tests in 0.083s

OK
Running micawber tests
All micawber tests passed
Skipping django tests
/nix/store/w66yxrq1y9abm15smji518sp6661qg0s-python2.7-micawber-0.3.4
/nix/store/d5b1wcda30g54s8k5gk1sl7zhdbl8gk8-python3.6-micawber-0.3.4

dotlambda
dotlambda reviewed Feb 14, 2018
View reviewed changes
pkgs/development/python-modules/micawber/default.nix Outdated
src = fetchFromGitHub {
owner = "coleifer";
repo = "micawber";
rev = "4754f0088f452c5227f808321a2abc38745dc248";
Copy link
Member

Choose a reason for hiding this comment

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

rev = version

Copy link
Member Author

Choose a reason for hiding this comment

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

changed. thanks!

pkgs/development/python-modules/micawber/default.nix
micawber supplies a few methods for retrieving rich metadata
about a variety of links, such as links to youtube videos.
micawber also provides functions for parsing blocks of text and html
and replacing links to videos with rich embedded content.
Copy link
Member

Choose a reason for hiding this comment

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

Even though I personally dislike not capitalizing the beginning of a sentence, I see that it's just copied from readthedocs. So I guess just leave it like that :)

Copy link
Member

Choose a reason for hiding this comment

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

From the guidelines:

Don’t include a period at the end. Don’t include newline characters. Capitalise the first character. For brevity, don’t repeat the name of package — just describe what it does.

Copy link
Member

Choose a reason for hiding this comment

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

That's for the description, not for longDescription.

@earldouglas earldouglas mentioned this pull request Feb 14, 2018
Merged
8 tasks
@davidak
Copy link
Member Author

davidak commented Feb 14, 2018

A new version was released on PyPI with tests included. Now it's perfect.

@davidak davidak changed the title micawber: init at 0.3.4 micawber: init at 0.3.5 Feb 14, 2018
@FRidh FRidh merged commit b234fa4 into NixOS:master Feb 18, 2018
@davidak davidak deleted the micawber branch March 10, 2018 14:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dotlambda dotlambda dotlambda approved these changes

@FRidh FRidh Awaiting requested review from FRidh

Assignees
No one assigned
Labels
6.topic: python 8.has: package (new) 10.rebuild-darwin: 1-10 10.rebuild-linux: 1-10
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@davidak @disassembler @dotlambda @GrahamcOfBorg @FRidh