-
Notifications
You must be signed in to change notification settings - Fork 10
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
Ghost points test still failing in some circumstances #28
Comments
Thank you very much for noticing this and reporting! I'll spend some time on this and see what I can see. |
After reading some code and thinking about this, I think the issue at hand is at a deeper level than a timestamp mis-sort at the osm-p2p-server layer. Consider the following: The test in question performs two forks on a way (the ORIGINAL WAY): one is a deletion of a point (the DELETED WAY), and the other is a modification of one point id to another (MODIFIED WAY). Since the test failure is reporting that a way returned contains a node that has already been deleted, it means that it's the ORIGINAL WAY that's being returned here, since neither the ORIGINAL WAY nor the DELETED WAY contain a deleted point. This should not be possible: both forks come causally after the ORIGINAL WAY, so there shouldn't be any means for that way to be returned anymore. The good news is that this relatively small test may provide us with the means to locate the squash the long standing "SKIP bug". |
Add debug output to test to track down #28.
It seems like the ghost points test still randomly fails: https://travis-ci.org/digidem/osm-p2p-server/builds/191945300#L2390
Notice the 'SKIP' message which is produced by this code: https://github.com/digidem/osm-p2p-server/blob/master/lib/check_ref_ex.js
The code already tries to replicate race condition issues by adding a random delay to database queries, so I would think this bug is more likely related to
id
orversion
sorting issues, given how rarely it seems to cause the test to fail.We really need to track down and fix the bug that required this skip code in the first place.
The text was updated successfully, but these errors were encountered: