Occasional incorrect ordering of feature.children during test of PUT to view_feature

RESOLVED FIXED

Status

Mozilla Developer Network
BrowserCompat
--
minor
RESOLVED FIXED
3 years ago
2 years ago

People

(Reporter: jwhitlock, Assigned: jwhitlock)

Tracking

Details

(Whiteboard: [type:bug][bc:infra])

Attachments

(1 attachment)

(Assignee)

Description

3 years ago
What did you do?
================
About 5% of the time, the test 'test_post_add_second_subfeature' in webplatform/tests/test_view_serializers.py will fail.

What happened?
==============
The test fails because the order of two child features is swapped.    The test has always succeeded on the second run.

What should have happened?
==========================
The expected order of child features is creation order - the existing child feature, then the newly added child feature

Is there anything else we should know?
======================================
Order should be determined by the django-mptt 3rd-party app.  This could be an issue with django-mptt 0.7.1, or with the code that handles updating of mptt fields after an update (see webplatformcompat/view_serializers.py, FeatureExtra.save).

This happens locally and in TravisCI.  It seems to happen in the Python 3.4 tests, but that could be faulty memory.  The test has succeeded on the second run, which had made debugging difficult.
(Assignee)

Updated

3 years ago
Blocks: 996570
Severity: normal → minor
OS: Other → All
(Assignee)

Comment 1

3 years ago
I just got the bug under Python 2.7, so it's not a 3.4 issue.
(Assignee)

Comment 2

3 years ago
I've added some code to dig in further (https://github.com/jwhitlock/web-platform-compat/commit/2f06ac5474526b376c5d0f51952370060b82feac) and was rewarded with a bug report:

======================================================================
Unexpected failure in test_post_add_second_subfeature.
Report at:
 https://bugzilla.mozilla.org/show_bug.cgi?id=1159337
parent: id=430, lft=1, rght=4, tree_id=1, level=0, parent=None
parent from DB: id=430, lft=1, rght=6, tree_id=1, level=0, parent=None
sf1: id=431, lft=2, rght=3, tree_id=1, level=1, parent=feature
sf2: id=432, lft=4, rght=5, tree_id=1, level=1, parent=feature
======================================================================

There is definitely an issue.  The parent rght field should be 6, and is when read directly from the database, but is the old value of 4 in the subject code.  More investigation is needed, starting with database traces.
(Assignee)

Updated

2 years ago
Assignee: nobody → jwhitlock
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true

Comment 3

2 years ago
Commits pushed to master at https://github.com/mozilla/web-platform-compat

https://github.com/mozilla/web-platform-compat/commit/8be53a3d395639dbcde5c4badca24246e24008cc
bug 1159337 - Debug occasional test failure

When the test test_post_add_second_subfeature results in an unexpected
feature order, print a bunch of debug data, including the MPTT state and
database queries, before failing the test. This debug code does not run
under normal API calls, only in the test.

https://github.com/mozilla/web-platform-compat/commit/c79b02f88b3bb6c8ab594d0ec12ed90a05903b06
Merge pull request #42 from jwhitlock/1159337_feature_ordering_bug

bug 1159337 - Debug occasional test failure
Component: General → BrowserCompat
(Assignee)

Comment 4

2 years ago
New instance, but DB queries didn't work.  The disconnect between the instance's parent.rght and the database's parent.rght appears again.

======================================================================
Unexpected failure in test_post_add_second_subfeature.
Report at:
 https://bugzilla.mozilla.org/show_bug.cgi?id=1159337
DB queries:
Features:
parent: id=340, lft=1, rght=4, tree_id=1, level=0, parent=None
parent from DB: id=340, lft=1, rght=6, tree_id=1, level=0, parent=None
sf1: id=341, lft=2, rght=3, tree_id=1, level=1, parent=feature
sf2: id=342, lft=4, rght=5, tree_id=1, level=1, parent=feature
======================================================================
(Assignee)

Comment 5

2 years ago
This occurred again intermittently while upgrading dependencies.  The parent.rght disconnect doesn't seem to be the issue - that's a result of Django caching instance values.  Mysterious.

Comment 6

2 years ago
Commits pushed to master at https://github.com/mdn/browsercompat

https://github.com/mdn/browsercompat/commit/303b1a1565de14520e4f645962e51ef555651ae4
bug 1159337 - Add DB queries for test failure

This code will capture the database queries, so that the queries for a
failing test can be compared to the passing test.

https://github.com/mdn/browsercompat/commit/f40af6cd42e5814ddd3e4c20aad8d7dbb534ac9f
Merge pull request #52 from mdn/debug_incorrect_order_1159337

bug 1159337 - Add DB queries for test failure
(Assignee)

Comment 7

2 years ago
Created attachment 8667901 [details]
bug_report.txt

Failure with DB queries

Comment 8

2 years ago
Commit pushed to master at https://github.com/mdn/browsercompat

https://github.com/mdn/browsercompat/commit/671344f4673bd4ac97a70cd0509274344dd5c346
fix bug 1159337 - Use TreeManager for Feature

Feature should be using a manager derived from django-mptt's
TreeManager, not the Django-standard manager.  Feature.children will now
return child features in tree order.

Updated

2 years ago
Status: ASSIGNED → RESOLVED
Last Resolved: 2 years ago
Resolution: --- → FIXED
Summary: [Compat Data] Occasional incorrect ordering of feature.children during test of PUT to view_feature → Occasional incorrect ordering of feature.children during test of PUT to view_feature
Whiteboard: [specification][type:bug] → [type:bug][bc:infra]
You need to log in before you can comment on or make changes to this bug.