Skip to content
This repository has been archived by the owner on Jun 1, 2024. It is now read-only.

Remove ./bin/trial #220

Closed
wants to merge 3 commits into from
Closed

Remove ./bin/trial #220

wants to merge 3 commits into from

Conversation

rodrigc
Copy link
Contributor

@rodrigc rodrigc commented Aug 18, 2016

@@ -523,7 +523,7 @@ class TwistedVirtualenvReactorsBuildFactory(TwistedBaseFactory):
treeStableTimer = 5*60

def __init__(self, source, RemovePYCs=RemovePYCs, python="python",
trial="./bin/trial", virtualenv_module="virtualenv",
trial="-m twisted.trial", virtualenv_module="virtualenv",
Copy link
Member

Choose a reason for hiding this comment

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

in this context, do we still need the trial argument at all?

I am assuming that all tests will now run with -m twisted.trial

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Maybe not, but I don't have access to the builders, so I am only guessing
at how the buildbot logic works by looking at it.

@adiroiban
Copy link
Member

I think that we need this, but instead of focusing on removing trial, we should focus on having all builders using tox to get a similar run as on travis/appeveyor.

In this way we will also get rid of trial.

We can start with removing a few builders like the one targeted in this PR.

With this change we should also cleanup the code which is no longer used ... ex remove TwistedVirtualenvReactorsBuildFactory

@rodrigc
Copy link
Contributor Author

rodrigc commented Aug 19, 2016

trial needs to go ASAP, because people still try to check out the code and use it in the tree.
Because of new dependencies on constantly and moving of the source code to src,
it no longer works the way it use to in the tree, leading to confusion.

I wanted to remove trial before, but was opposed during code review, due to the code in the buildbots which use it.

@adiroiban
Copy link
Member

we can run buildmaster/buildslave in a local vagrant VM for testing/development ... and this is how all buildbot changes should be done before deployment

here is some documentation https://github.com/twisted-infra/braid#vagrant

all buildbot configuration is public and is defined here https://github.com/twisted-infra/braid/blob/master/services/buildbot/master/

@rodrigc
Copy link
Contributor Author

rodrigc commented Aug 20, 2016

It's not clear to me how to use this vagrant setup to test buildbot. I tried following the steps
and was prompted for a passphrase when running commands like fab config.vagrant buildbot.restart. Also, the vagrant instructions do not make it obvious how to test changes to Windows buildbot configurations.

@adiroiban
Copy link
Member

testing windows slaves is not yet supported

if if asked your for a password, then the vagrant vm was not set up ok... can you do vagrant ssh to the machine?

there are a couple of tickets targeting the removal of non-tox builders:

but I think that first we should have https://twistedmatrix.com/trac/ticket/8544 this implemented and merged

@adiroiban
Copy link
Member

so on https://twistedmatrix.com/trac/ticket/8544 due to pypa/virtualenv#929 I have a bad time implementing tox/venv on Windows

and this branch is doing the same... trying to run tox/venv on Windows to get rid of bin/trial

we might want to change strategy and remove the windows builds from buildbot and only have them on appveyor ... but appveyor is windows2012r2 ... and we are no generating the release wheel on win7 (win2008) ... so not sure if this will have an impact

I will do one more push for the venv --clear workaround on windows

@glyph
Copy link
Contributor

glyph commented Mar 30, 2017

Is this still relevant?

@rodrigc
Copy link
Contributor Author

rodrigc commented Mar 30, 2017

It looks like all the builds have been converted to use Tox in master.cfg.
The code which references ./bin/trial can probably go away because those steps are not used.

@rodrigc rodrigc closed this Mar 30, 2017
@rodrigc rodrigc deleted the 8765-rodrigc-trial-removal branch March 30, 2017 14:44
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants