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

Reorganize and clean up tests #87

Merged
merged 23 commits into from
Dec 17, 2018
Merged

Reorganize and clean up tests #87

merged 23 commits into from
Dec 17, 2018

Conversation

dhimmel
Copy link
Member

@dhimmel dhimmel commented Dec 13, 2018

Places tests closer to the code they're testing in the project's directory tree.

This PR does not create new tests, just moves existing tests to the new directory structure. Tests should functionally be the same.

Places tests closer to the code they're testing in the project's
directory tree.
@dhimmel dhimmel changed the title Move tests in package directory Reorganize and clean up tests Dec 13, 2018
@dhimmel dhimmel requested a review from agitter December 13, 2018 22:09
@dhimmel dhimmel force-pushed the test-reorg branch 2 times, most recently from 67b1224 to d38569b Compare December 14, 2018 17:12
@dhimmel
Copy link
Member Author

dhimmel commented Dec 14, 2018

Builds are failing due to an intermittent 504 bad-gateway error on the translation-server:
https://ci.appveyor.com/project/greenelab/manubot/builds/21018471/job/yd35t230v6ni6ty8#L186

CC @dongbohu

@agitter
Copy link
Member

agitter commented Dec 14, 2018

The restructuring looks good to me except for the error you noted above.

@dongbohu
Copy link

I tried this command:

curl -d https://bigthink.com/neurobonkers/a-pirate-bay-for-science -H 'Content-Type: text/plain' http://translate.manubot.org/web

and got the output:

[
  {
    "key": "8V3UIHBX",
    "version": 0,
    "itemType": "webpage",
    "creators": [],
    "tags": [],
    "title": "Meet the Robin Hood of Science",
    "websiteTitle": "Big Think",
    "date": "2016-02-09T20:12:00",
    "url": "https://bigthink.com/neurobonkers/a-pirate-bay-for-science",
    "abstractNote": "How one researcher created a pirate bay for science more powerful than even libraries at top universities.",
    "language": "en",
    "accessDate": "2018-12-14T18:43:33Z"
  }
]

@dhimmel dhimmel force-pushed the test-reorg branch 2 times, most recently from 8be9d52 to a6a8ea9 Compare December 15, 2018 14:53
@dhimmel
Copy link
Member Author

dhimmel commented Dec 16, 2018

pytest manubot/cite/tests/test_cite_command.py::test_cite_command_render_stdout

is failing because DataCite is down again. AppVeyor did not fail because these tests are skipped, which is bizzare. Pandoc is now version 2.5 on AppVeyor, but don't see why that would skip the test.

@dhimmel
Copy link
Member Author

dhimmel commented Dec 16, 2018

de2ef52 added the -rs flag to pytest to show why tests are skipped. Result

SKIP [5] manubot\cite\tests\test_cite_command.py:52: pandoc-citeproc installation not found on system

@jgm in the past, the following commands installed pandoc and pandoc-citeproc on windows:

https://github.com/greenelab/manubot/blob/26a6fbd91ce70a1f8a424df1d3b8c7354afdfa77/.appveyor.yml#L20-L21

Has something changed? Any advice?

@agitter
Copy link
Member

agitter commented Dec 16, 2018

@dhimmel I believe you want this line for the pandoc install from chocolatey command

https://github.com/greenelab/manubot/blob/26a6fbd91ce70a1f8a424df1d3b8c7354afdfa77/.appveyor.yml#L25

The chocolatey pandoc package maintainer has a GitHub profile if we can't debug this.

I reviewed the AppVeyor build log, and it looks like pandoc is installed to a different path now.

Originally:

The install of pandoc was successful.
  Software installed to 'C:\Program Files (x86)\Pandoc\'
Chocolatey installed 2/2 packages. 
 See the log for details (C:\ProgramData\chocolatey\logs\chocolatey.log).
$env:Path += ";C:\Program Files (x86)\Pandoc\"

Currently:

The install of pandoc was successful.
  Software installed to 'C:\Program Files\Pandoc\'
Chocolatey installed 2/2 packages. 
 See the log for details (C:\ProgramData\chocolatey\logs\chocolatey.log).
$env:Path += ";C:\Program Files (x86)\Pandoc\"

Can you try updating
https://github.com/greenelab/manubot/blob/26a6fbd91ce70a1f8a424df1d3b8c7354afdfa77/.appveyor.yml#L26
to use C:\Program Files\Pandoc\?

@dhimmel
Copy link
Member Author

dhimmel commented Dec 17, 2018

I reviewed the AppVeyor build log, and it looks like pandoc is installed to a different path now.

Thanks @agitter... that fixed the AppVeyor issue. I also switched to specifying which version of pandoc in choco install for AppVeyor. Also removed the DataCite DOI from the cite command tests because the uptime of DataCite DOI Content Negotiation has been so poor. Hopefully, these changes will make test_cite_command_render_stdout less subject to whimsical failures.

Copy link
Member

@agitter agitter left a comment

Choose a reason for hiding this comment

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

With your latest changes, this looks good to me.

@dhimmel dhimmel merged commit 15eae64 into manubot:master Dec 17, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants