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

move some code and make nodeid computed early #3291

Merged
merged 7 commits into from
Mar 12, 2018
Merged

move some code and make nodeid computed early #3291

merged 7 commits into from
Mar 12, 2018

Conversation

RonnyPfannschmidt
Copy link
Member

@RonnyPfannschmidt RonnyPfannschmidt commented Mar 9, 2018 •
edited
Loading

@nicoddemus i had some local failures wrt progress display on xdist, if that shows up on travis as well please have a look

@RonnyPfannschmidt RonnyPfannschmidt changed the base branch from master to features March 9, 2018 10:06
@coveralls
Copy link

coveralls commented Mar 9, 2018 •
edited
Loading

Coverage Status

Coverage increased (+0.06%) to 92.726% when pulling 0302622 on RonnyPfannschmidt:small-moves into d6ddeb3 on pytest-dev:features.

nicoddemus
nicoddemus approved these changes Mar 9, 2018
View reviewed changes
Copy link
Member

@nicoddemus nicoddemus left a comment

Choose a reason for hiding this comment

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

@RonnyPfannschmidt this looks fine, I'm curious why you think it is better to be able to pass in the nodeid instead of having it computed always (I have a few reasons but I would like to hear yours first 😁).

Also, I guess this warrants a trivial CHANGELOG entry.

_pytest/nodes.py
@@ -319,31 +282,34 @@ def _prunetraceback(self, excinfo):
excinfo.traceback = ntraceback.filter()


def _check_initialpaths_for_relpath(session, fspath):
Copy link
Member

Choose a reason for hiding this comment

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

👍

@RonnyPfannschmidt
Copy link
Member Author

one of the reasons is that it makes introduction of new nodes and note types much more painful
in future i think we need to change how nodes are constructed, the smart ctors where a massive mistake that makes structural change a hazard (i need various new types of nodes to express marks in parametrization sanely)

the other thing is that that we basically had a critical value for node identification generated "magically at a distance" - thats just rotten bad engineering

@nicoddemus nicoddemus merged commit 0557ab4 into pytest-dev:features Mar 12, 2018
@RonnyPfannschmidt RonnyPfannschmidt deleted the small-moves branch October 1, 2018 09:34
@Cito Cito mentioned this pull request Nov 13, 2021
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nicoddemus nicoddemus nicoddemus 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.

None yet
3 participants
@RonnyPfannschmidt @coveralls @nicoddemus