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

Refactor test dependencies #2351

Merged
merged 3 commits into from
Dec 5, 2016
Merged

Refactor test dependencies #2351

merged 3 commits into from
Dec 5, 2016

Conversation

ardhipoetra
Copy link

@ardhipoetra ardhipoetra commented Jun 29, 2016 •
edited
Loading

In this PR, I will fix dependencies on tests that I can find. This also related to this PR

@ardhipoetra ardhipoetra changed the title Refactor test dependencies WIP: Refactor test dependencies Jun 29, 2016
@ardhipoetra ardhipoetra force-pushed the refactor_test_dependencies branch 9 times, most recently from 1202c63 to 069fb18 Compare July 2, 2016 06:45
@ardhipoetra
Copy link
Author

you okay with this changes, @devos50?

@ardhipoetra ardhipoetra force-pushed the refactor_test_dependencies branch from b5e15f0 to 93cb96d Compare July 5, 2016 03:24
@ardhipoetra ardhipoetra force-pushed the refactor_test_dependencies branch 3 times, most recently from aa037bf to d0407a9 Compare July 13, 2016 09:08
@ardhipoetra
Copy link
Author

retest this please

@ardhipoetra ardhipoetra changed the title WIP: Refactor test dependencies DON'T MERGE: Refactor test dependencies Jul 13, 2016
@ardhipoetra ardhipoetra changed the title DON'T MERGE: Refactor test dependencies WIP: Refactor test dependencies Jul 13, 2016
@ardhipoetra
Copy link
Author

I'm also finished with this one, @devos50. Or maybe can you point out other test that have dependencies?

@ardhipoetra
Copy link
Author

retest this please

@ardhipoetra
Copy link
Author

retest this please

last run : timeout on TestVariablesEndpoint

devos50
devos50 reviewed Jul 18, 2016
View reviewed changes
Tribler/Test/Core/CreditMining/test_creditmining_sys.py
@@ -20,8 +20,9 @@
from Tribler.Main.Utility.GuiDBTuples import CollectedTorrent
from Tribler.Policies.BoostingManager import BoostingManager, BoostingSettings
from Tribler.Test.Core.CreditMining.mock_creditmining import MockLtTorrent, ResourceFailClass
from Tribler.Test.common import TORRENT_FILE
from Tribler.Test.test_as_server import TestAsServer, TESTS_DATA_DIR
from Tribler.Test.Core.Modules.Channel.base_test_channel import BaseTestChannel
Copy link
Contributor

@devos50 devos50 Jul 18, 2016 •
edited
Loading

Choose a reason for hiding this comment

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

Here you are creating a dependency again between Tribler.Test.Core.Modules.Channel and Tribler.Test.Core.CreditMining. Please try to avoid that (i.e. moving the code you need to another location).

Copy link
Author

Choose a reason for hiding this comment

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

Actually, that's where I put common function that used in Both Core.CreditMining and Core.Modules.Channel. Should I put that, let say, in Test.Core?

@ardhipoetra ardhipoetra force-pushed the refactor_test_dependencies branch from 3b92877 to fecca2b Compare July 20, 2016 10:19
@ardhipoetra
Copy link
Author

retest this please

last run error in TestModifyChannelTorrentEndpoint

@synctext synctext mentioned this pull request Sep 21, 2016
Closed
@devos50
Copy link
Contributor

devos50 commented Nov 30, 2016

@ardhipoetra what's the status of this PR?

@ardhipoetra ardhipoetra force-pushed the refactor_test_dependencies branch 2 times, most recently from 9225cdb to 65525e1 Compare November 30, 2016 14:48
@ardhipoetra
Copy link
Author

ardhipoetra commented Nov 30, 2016 •
edited
Loading

@devos50, last time I worked on it, it was ready for review.

Now some are conflicting, I'm on my way to fix them.

@ardhipoetra ardhipoetra force-pushed the refactor_test_dependencies branch from 9a0f722 to 19667dc Compare November 30, 2016 15:16
@ardhipoetra ardhipoetra force-pushed the refactor_test_dependencies branch 5 times, most recently from 4578209 to abc7894 Compare November 30, 2016 18:27
@ardhipoetra
Copy link
Author

retest this please

last run : looks okay

@ardhipoetra
Copy link
Author

retest this please

@ardhipoetra
Copy link
Author

Do you want to review it again, @devos50?

@synctext
Copy link
Member

retest this please

2 similar comments
@synctext
Copy link
Member

retest this please

@ardhipoetra
Copy link
Author

retest this please

@ardhipoetra ardhipoetra changed the title WIP: Refactor test dependencies READY: Refactor test dependencies Dec 5, 2016
@devos50
Copy link
Contributor

devos50 commented Dec 5, 2016

@ardhipoetra looks ok! Please squash the fixup commits and I will merge it.

@ardhipoetra ardhipoetra force-pushed the refactor_test_dependencies branch from abc7894 to 12989aa Compare December 5, 2016 12:05
@ardhipoetra ardhipoetra force-pushed the refactor_test_dependencies branch from 12989aa to 5a74831 Compare December 5, 2016 12:12
@ardhipoetra
Copy link
Author

retest this please

1 similar comment
@ardhipoetra
Copy link
Author

retest this please

@ardhipoetra
Copy link
Author

go on and merge, @devos50

@devos50 devos50 changed the title READY: Refactor test dependencies Refactor test dependencies Dec 5, 2016
@devos50 devos50 merged commit cd3ca03 into Tribler:devel Dec 5, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@ardhipoetra @devos50 @synctext