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

Add cwd back to sys.path for trial #672

Closed
wants to merge 6 commits into from
Closed

Add cwd back to sys.path for trial #672

wants to merge 6 commits into from

Conversation

rodrigc
Copy link
Contributor

@rodrigc rodrigc commented Jan 12, 2017

@exarkun
Copy link
Member

exarkun commented Jan 12, 2017

Note I closed the referenced Twisted ticket as a duplicate of one filed a few days earlier. Also it's not clear this behavior should be restored since various people have implied that the change was intentional.

@rodrigc
Copy link
Contributor Author

rodrigc commented Jan 12, 2017

Well, I did the work to remove bin/trial from the system,
so the change of not adding cwd to sys.path for trial was unintentional on my part.

@rodrigc
Copy link
Contributor Author

rodrigc commented Jan 12, 2017

The other thing that was done in the Twisted code tree to deal with the path of things
was to move everything to the src directory in : 59709f6 for http://twistedmatrix.com/trac/ticket/8214

That affects testing and working on the Twisted codebase itself.
This fix doesn't affect the work done for ticket 8214.
It also restores behavior introduced in cb98a91
that apparently people who use trial to test their own code were depending on.

@codecov-io
Copy link

codecov-io commented Jan 12, 2017

Current coverage is 91.15% (diff: 100%)

Merging #672 into trunk will decrease coverage by 0.06%

@@              trunk       #672   diff @@
==========================================
  Files           838        838          
  Lines        146895     146913    +18   
  Methods           0          0          
  Messages          0          0          
  Branches      12980      12980          
==========================================
- Hits         133999     133918    -81   
- Misses        10664      10750    +86   
- Partials       2232       2245    +13   

Powered by Codecov. Last update e90982d...75627f8

@rodrigc
Copy link
Contributor Author

rodrigc commented Jan 29, 2017

Based on complaints I have seen on the mailing list, I think this behavior should be
restored for trial. There are some people who use trial for testing their own code
(not Twisted itself), and the change in behavior that occurred is annoying.
I missed it when I converted trial to a console script, because all the Twisted unit tests passed.

I added a new unit test for this behavior, so it will be noticeable if this breaks in future.

Copy link
Member

@Julian Julian left a comment

Choose a reason for hiding this comment

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

lgtm, appreciate you hitting this given me being one of afformentioned-slightly-annoyed people :P

@Julian
Copy link
Member

Julian commented Feb 9, 2017

Erm, could you address what @exarkun pointed out in some way though with ticket logistics? Gonna just consider this relevant to 8972 given that it's the open ticket, but maybe you should just move the topfile or something.

@exarkun
Copy link
Member

exarkun commented Feb 9, 2017

@Julian
Copy link
Member

Julian commented Feb 12, 2017

@rodrigc sounds like consensus was to WONTFIX then -- can you close?

@hawkowl
Copy link
Member

hawkowl commented Aug 7, 2017

This is a won't fix, yes.

@hawkowl hawkowl closed this Aug 7, 2017
@hawkowl hawkowl deleted the fix-trial-trunk branch August 7, 2017 01:51
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

6 participants