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

Daemon should respect the rule that stubs are normal even with follow-imports=skip #5566

Closed
ilevkivskyi opened this issue Sep 3, 2018 · 8 comments

Comments

@ilevkivskyi
Copy link
Member

Currently this test fails, presumably because the daemon applies --follow-imports=skip to stub files as well (while it shouldn't):

[case testFineFollowImportSkipInvalidatedOnAddedStub]
# flags: --follow-imports=skip --ignore-missing-imports
# cmd: mypy main.py
[file main.py]
import other
x: str = other.x
[file other.pyi.2]
x = 1
[stale main, other]
[rechecked main, other]
[out]
==
main:2: error: Incompatible types in assignment (expression has type "int", variable has type "str")

This was found in #5556

@msullivan do you have any ideas why this may happen?

@ilevkivskyi
Copy link
Member Author

It looks like same happens with --bazel incremental build.

@gvanrossum could you give some hints about why this might happen?

@gvanrossum
Copy link
Member

(The thing we saw internally with Bazel has a completely different cause, unrelated to this issue.)

@ilevkivskyi
Copy link
Member Author

Yes, I just tried to look a bit more at this, and it looks like the root cause is simple actually: daemon never supposed to work with follow_imports=normal, but apparently it should always do this for stubs.

@JukkaL what do you think about this?

Also it looks like I had some misunderstanding about how fine-grained tests work, it seems to me now that by default at every test step all the modules currently present are passed to Server.check(). This means I have previously added many tests like [case testFineAddedMissingStubs] that don't test what I wanted.

@JukkaL
Copy link
Collaborator

JukkaL commented Sep 5, 2018

@ilevkivskyi Hmm this is pretty unfortunate. On the other hand, this has been around for a long time before this got reported, so it doesn't seem that an urgent fix is needed. We can look at this in a few weeks when the core team will have a bug fixing spree.

@msullivan
Copy link
Collaborator

I've looked into this some and it is no easier to properly support follow_imports=normal for stubs alone than it is to support it in general. That makes this a sub-issue of the newly opened #5870.

@msullivan msullivan removed their assignment Jun 18, 2020
@msullivan
Copy link
Collaborator

This might be easier to fix now with the new follow-imports=normal stuff?

@JukkaL
Copy link
Collaborator

JukkaL commented Jun 19, 2020

This might be easier now, though I can't say how easy exactly without spending some time on this.

@JukkaL
Copy link
Collaborator

JukkaL commented Mar 5, 2021

This works now.

@JukkaL JukkaL closed this as completed Mar 5, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants