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

Fix for #292 and changed filenamelimit #293

Open
wants to merge 7 commits into
base: master
Choose a base branch
from
Open

Fix for #292 and changed filenamelimit #293

wants to merge 7 commits into from

Conversation

rfelten
Copy link

@rfelten rfelten commented Feb 22, 2017

See #292. Any comments are welcome.

@rfelten rfelten mentioned this pull request Feb 22, 2017
Open
@rfelten
Copy link
Author

rfelten commented Mar 1, 2017

PR not ok? Anything I should change before merging? Please note that build fails due to other reasons, see https://travis-ci.org/WikiTeam/wikiteam/builds.

Hydriz
Hydriz reviewed Mar 2, 2017
View reviewed changes
testing/test_dumpgenerator_offline.py
class TestDumpgeneratorOffline(unittest.TestCase):

def setUp(self):
other = dict() # FIXME: get from dumpgenerator, but code base is a pre-OO mess
Copy link
Member

Choose a reason for hiding this comment

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

+1 for inspirational quote. Wonderful poetry that summarizes dumpgenerator.py in a single sentence.

Copy link
Author

Choose a reason for hiding this comment

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

hehe, sry, could not resist ;)
I was pretty discouraged after giving up to get the configuration out of dumpgenerator.py :(

Hydriz
Hydriz approved these changes Mar 2, 2017
View reviewed changes
Copy link
Member

@Hydriz Hydriz left a comment

Choose a reason for hiding this comment

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

Your change looks good, but the test case that you implemented does not seem to be picked up by Travis CI. Can you have a look at that?

@rfelten
Copy link
Author

rfelten commented Mar 2, 2017

Thanks for your feedback @Hydriz. For me it looks like the build fails due to other reasons? The PR introduces test_dumpgenerator_offline.py, but travis seems to fail on test cases on /test_dumpgenerator.py:

  File "/home/travis/build/WikiTeam/wikiteam/testing/test_dumpgenerator.py", line 195, in test_getPageTitles
    self.assertEqual(len(result_api), len(result_index))
AssertionError: 2247 != 1867

 File "/home/travis/build/WikiTeam/wikiteam/testing/test_dumpgenerator.py", line 270, in test_getWikiEngine
    self.assertEqual(guess_engine, engine)
AssertionError: 'Unknown' != 'TWiki'

@nemobis
Copy link
Member

nemobis commented Feb 10, 2020

I'll test this in the upcoming days. Make now any change you want me to test!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Hydriz Hydriz Hydriz approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@rfelten @nemobis @Hydriz