New issue
Advanced search Search tips
Starred by 3 users

Issue metadata

Status: Released
Owner: ----
Closed: Apr 11
Cc:
Components:



Sign in to add a comment

Starred changes based on elasticsearch index do not show as such in change list views

Project Member Reported by marco.mm...@gmail.com, Mar 19

Issue description

*****************************************************************
*****                                                       *****
***** !!!! THIS BUG TRACKER IS FOR GERRIT CODE REVIEW !!!!  *****
*****                                                       *****
***** DO NOT SUBMIT BUGS FOR CHROME, ANDROID, CYANOGENMOD,  *****
***** INTERNAL ISSUES WITH YOUR COMPANY'S GERRIT SETUP, ETC.*****
*****                                                       *****
*****   THOSE ISSUES BELONG IN DIFFERENT ISSUE TRACKERS     *****
*****                                                       *****
*****************************************************************

Affected Version: 2.14

What steps will reproduce the problem?
1. Initialize a gerrit site with elasticsearch index type (not default lucene).
2. Create a test project then push a change for review.
3. Star the change.

What is the expected output?
The star icon should be highlighted as starred for that change in all views - change lists and change detail.

What do you see instead?
The star icon shows as highlighted (starred change) only in the change detail view or page.
Lists that include that change do not show it as starred through that icon, which remains turned off.

Please provide any additional information below.
N/a.
 
The acceptance tests include tests for starred changes.  We should try to expand those to expose this issue.

Project Member

Comment 2 by marco.mm...@gmail.com, Apr 11

Status: ChangeUnderReview (was: New)
https://gerrit-review.googlesource.com/c/gerrit/+/171470

Project Member

Comment 3 by marco.mm...@gmail.com, Apr 11

The author of that change is about to look into the above comment, now.
Labels: FixedIn-2.14.8
Status: Submitted (was: ChangeUnderReview)
Project Member

Comment 5 by borui....@gmail.com, Apr 12

Hello David:

I tried to modify the acceptance tests (AbstractQueryChangesTest.byStar) such that if I comment out my fix for this issue, the test should fail. However, after I investigated more on both sites (Lucene & Elasticsearch), I found that this test would pass if I comment out the code that set the "star" field of the change in the change index. Not only this is true for this test on both sites, but also the other tests (for example AbstractQueryChangesTest.bySize) would pass if I comment the code that set the "added" and "deleted" fields in the change index.

So my question is whether it is necessary that I modify the test. In my opinion, these tests (byStar or bySize) do not validate the behavior of the ChangeIndex class (Elasticsearch & Lucene). If I don't make it clear please let me know :) Thank you.

Project Member

Comment 6 by marco.mm...@gmail.com, Apr 17

Components: TestCoverage
I'm adding the TestCoverage component to this closed issue, just to make that aspect of it explicit.

Borui and us at least are not pursuing this farther, as per the previous comment, unless further notice that is.
Status: Released (was: Submitted)

Sign in to add a comment