Skip to content

Bug 1923923 - Allow creation or update (annotation) of an internal issue #8532

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

Open
wants to merge 21 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 1 addition & 2 deletions treeherder/config/settings.py
Original file line number Diff line number Diff line change
Expand Up @@ -448,9 +448,8 @@
MAX_ERROR_LINES = 40
FAILURE_LINES_CUTOFF = 150

# Required number of occurrences in a given time window before being prompted to file a bug in Bugzilla
# Count internal issue annotations in a limited time window (before prompting user to file a bug in Bugzilla)
INTERNAL_OCCURRENCES_DAYS_WINDOW = 7
INTERNAL_OCCURRENCES_COUNT = 3

# Perfherder
# Default minimum regression threshold for perfherder is 2% (otherwise
Expand Down
2 changes: 1 addition & 1 deletion treeherder/model/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -261,7 +261,7 @@ def serialize(self):
attrs["occurrences"] = None
if attrs["id"] is None:
# Only fetch occurrences for internal issues. It causes one extra query
attrs["occurrences"] = BugJobMap.objects.filter(
attrs["occurrences"] = self.jobmap.filter(
created__gte=timezone.now()
- datetime.timedelta(days=settings.INTERNAL_OCCURRENCES_DAYS_WINDOW)
).count()
Expand Down
14 changes: 14 additions & 0 deletions treeherder/webapp/api/internal_issue.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
from rest_framework import generics

from treeherder.webapp.api.serializers import InternalIssueSerializer


class CreateInternalIssue(generics.CreateAPIView):
"""
Create a Bugscache entry, not necessarilly linked to a real Bugzilla ticket.
In case it already exists, update its occurrences.

Returns the number of occurrences of this bug in the last week.
"""

serializer_class = InternalIssueSerializer
20 changes: 20 additions & 0 deletions treeherder/webapp/api/serializers.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

from django.contrib.auth.models import User
from django.core.exceptions import ObjectDoesNotExist
from django.utils import timezone
from rest_framework import serializers

from treeherder.changelog.models import Changelog
Expand Down Expand Up @@ -446,3 +447,22 @@ class InvestigatedTestsSerializers(serializers.ModelSerializer):
class Meta:
model = models.InvestigatedTests
fields = ("id", "test", "job_name", "job_symbol")


class InternalIssueSerializer(serializers.ModelSerializer):
internal_id = serializers.IntegerField(source="id", read_only=True)

class Meta:
model = models.Bugscache
fields = ("internal_id", "summary")

def create(self, validated_data):
"""Build or retrieve a bug already reported for a similar FailureLine"""
try:
bug, _ = models.Bugscache.objects.get_or_create(
**validated_data, defaults={"modified": timezone.now()}
)
except models.Bugscache.MultipleObjectsReturned:
# Take last modified in case a conflict happens
bug = models.Bugscache.objects.filter(**validated_data).order_by("modified").first()
return bug
4 changes: 4 additions & 0 deletions treeherder/webapp/api/urls.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
groups,
infra_compare,
intermittents_view,
internal_issue,
investigated_test,
job_log_url,
jobs,
Expand Down Expand Up @@ -175,4 +176,7 @@
),
re_path(r"^csp-report/$", csp_report.csp_report_collector, name="csp-report"),
re_path(r"^schema/", get_schema_view(title="Treeherder Rest API"), name="openapi-schema"),
re_path(
r"^internal_issue/", internal_issue.CreateInternalIssue.as_view(), name="internal_issue"
),
]
76 changes: 76 additions & 0 deletions ui/helpers/bug.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,76 @@
export const omittedLeads = [
'TEST-UNEXPECTED-FAIL',
'PROCESS-CRASH',
'TEST-UNEXPECTED-ERROR',
'REFTEST ERROR',
];
/*
* Find the first thing in the summary line that looks like a filename.
*/
const findFilename = (summary) => {
// Take left side of any reftest comparisons, as the right side is the reference file
// eslint-disable-next-line prefer-destructuring
summary = summary.split('==')[0];
// Take the leaf node of unix paths
summary = summary.split('/').pop();
// Take the leaf node of Windows paths
summary = summary.split('\\').pop();
// Remove leading/trailing whitespace
summary = summary.trim();
// If there's a space in what's remaining, take the first word
// eslint-disable-next-line prefer-destructuring
summary = summary.split(' ')[0];
return summary;
};
/*
* Remove extraneous junk from the start of the summary line
* and try to find the failing test name from what's left
*/
export const parseSummary = (suggestion) => {
let summary = suggestion.search;
const searchTerms = suggestion.search_terms;
// Strip out some extra stuff at the start of some failure paths
let re = /file:\/\/\/.*?\/build\/tests\/reftest\/tests\//gi;
summary = summary.replace(re, '');
re = /chrome:\/\/mochitests\/content\/a11y\//gi;
summary = summary.replace(re, '');
re = /http:\/\/([0-9]+\.[0-9]+\.[0-9]+\.[0-9]+):([0-9]+)\/tests\//gi;
summary = summary.replace(re, '');
re = /xpcshell([-a-zA-Z0-9]+)?.(ini|toml):/gi;
summary = summary.replace(re, '');
summary = summary.replace('/_mozilla/', 'mozilla/tests/');
// We don't want to include "REFTEST" when it's an unexpected pass
summary = summary.replace(
'REFTEST TEST-UNEXPECTED-PASS',
'TEST-UNEXPECTED-PASS',
);
const summaryParts = summary.split(' | ');

// If the search_terms used for finding bug suggestions
// contains any of the omittedLeads, that lead is needed
// for the full string match, so don't omit it in this case.
// If it's not needed, go ahead and omit it.
if (searchTerms.length && summaryParts.length > 1) {
omittedLeads.forEach((lead) => {
if (!searchTerms[0].includes(lead) && summaryParts[0].includes(lead)) {
summaryParts.shift();
}
});
}

// Some of the TEST-FOO bits aren't removed from the summary,
// so we sometimes end up with them instead of the test path here.
const summaryName =
summaryParts[0].startsWith('TEST-') && summaryParts.length > 1
? summaryParts[1]
: summaryParts[0];
const possibleFilename = findFilename(summaryName);

return [summaryParts, possibleFilename];
};

export const getCrashSignatures = (failureLine) => {
const crashRegex = /(\[@ .+\])/g;
const crashSignatures = failureLine.search.match(crashRegex);
return crashSignatures ? [crashSignatures[0]] : [];
};
3 changes: 3 additions & 0 deletions ui/helpers/constants.js
Original file line number Diff line number Diff line change
Expand Up @@ -361,3 +361,6 @@ export const sxsJobTypeName = 'perftest-linux-side-by-side';
export const sxsTaskName = 'side-by-side';

export const geckoProfileTaskName = 'geckoprofile';

// Number of internal issue classifications to open a bug in Bugzilla
export const requiredInternalOccurrences = 3;
16 changes: 10 additions & 6 deletions ui/job-view/details/PinBoard.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -358,16 +358,20 @@ class PinBoard extends React.Component {
);
};

isNumber = (text) => !text || /^[0-9]*$/.test(text);
isValidBugNumber = (text) => !text || /^i?[0-9]*$/.test(text);

saveEnteredBugNumber = () => {
const { newBugNumber, enteringBugNumber } = this.state;

if (enteringBugNumber) {
if (!newBugNumber) {
this.toggleEnterBugNumber(false);
} else if (this.isNumber(newBugNumber)) {
this.props.addBug({ id: parseInt(newBugNumber, 10) });
} else if (this.isValidBugNumber(newBugNumber)) {
if (newBugNumber[0] === 'i') {
this.props.addBug({ internal_id: newBugNumber.slice(1) });
} else {
this.props.addBug({ id: parseInt(newBugNumber, 10) });
}
this.toggleEnterBugNumber(false);
}
}
Expand Down Expand Up @@ -491,7 +495,7 @@ class PinBoard extends React.Component {
pattern="[0-9]*"
className="add-related-bugs-input"
placeholder="enter bug number"
invalid={!this.isNumber(newBugNumber)}
invalid={!this.isValidBugNumber(newBugNumber)}
onKeyPress={this.bugNumberKeyPress}
onChange={(ev) => {
this.setState({ newBugNumber: ev.target.value });
Expand Down Expand Up @@ -712,8 +716,8 @@ PinBoard.propTypes = {
isStaff: PropTypes.bool.isRequired,
isPinBoardVisible: PropTypes.bool.isRequired,
pinnedJobs: PropTypes.shape({}).isRequired,
pinnedJobBugs: PropTypes.shape({}).isRequired,
newBug: PropTypes.string.isRequired,
pinnedJobBugs: PropTypes.arrayOf(PropTypes.shape({})).isRequired,
newBug: PropTypes.shape({}).isRequired,
addBug: PropTypes.func.isRequired,
removeBug: PropTypes.func.isRequired,
unPinJob: PropTypes.func.isRequired,
Expand Down
82 changes: 3 additions & 79 deletions ui/shared/BugFiler.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -27,79 +27,9 @@ import {
getApiUrl,
} from '../helpers/url';
import { create } from '../helpers/http';
import { omittedLeads, parseSummary, getCrashSignatures } from '../helpers/bug';
import { notify } from '../job-view/redux/stores/notifications';

const omittedLeads = [
'TEST-UNEXPECTED-FAIL',
'PROCESS-CRASH',
'TEST-UNEXPECTED-ERROR',
'REFTEST ERROR',
];
/*
* Find the first thing in the summary line that looks like a filename.
*/
const findFilename = (summary) => {
// Take left side of any reftest comparisons, as the right side is the reference file
// eslint-disable-next-line prefer-destructuring
summary = summary.split('==')[0];
// Take the leaf node of unix paths
summary = summary.split('/').pop();
// Take the leaf node of Windows paths
summary = summary.split('\\').pop();
// Remove leading/trailing whitespace
summary = summary.trim();
// If there's a space in what's remaining, take the first word
// eslint-disable-next-line prefer-destructuring
summary = summary.split(' ')[0];
return summary;
};
/*
* Remove extraneous junk from the start of the summary line
* and try to find the failing test name from what's left
*/
const parseSummary = (suggestion) => {
let summary = suggestion.search;
const searchTerms = suggestion.search_terms;
// Strip out some extra stuff at the start of some failure paths
let re = /file:\/\/\/.*?\/build\/tests\/reftest\/tests\//gi;
summary = summary.replace(re, '');
re = /chrome:\/\/mochitests\/content\/a11y\//gi;
summary = summary.replace(re, '');
re = /http:\/\/([0-9]+\.[0-9]+\.[0-9]+\.[0-9]+):([0-9]+)\/tests\//gi;
summary = summary.replace(re, '');
re = /xpcshell([-a-zA-Z0-9]+)?.(ini|toml):/gi;
summary = summary.replace(re, '');
summary = summary.replace('/_mozilla/', 'mozilla/tests/');
// We don't want to include "REFTEST" when it's an unexpected pass
summary = summary.replace(
'REFTEST TEST-UNEXPECTED-PASS',
'TEST-UNEXPECTED-PASS',
);
const summaryParts = summary.split(' | ');

// If the search_terms used for finding bug suggestions
// contains any of the omittedLeads, that lead is needed
// for the full string match, so don't omit it in this case.
// If it's not needed, go ahead and omit it.
if (searchTerms.length && summaryParts.length > 1) {
omittedLeads.forEach((lead) => {
if (!searchTerms[0].includes(lead) && summaryParts[0].includes(lead)) {
summaryParts.shift();
}
});
}

// Some of the TEST-FOO bits aren't removed from the summary,
// so we sometimes end up with them instead of the test path here.
const summaryName =
summaryParts[0].startsWith('TEST-') && summaryParts.length > 1
? summaryParts[1]
: summaryParts[0];
const possibleFilename = findFilename(summaryName);

return [summaryParts, possibleFilename];
};

export class BugFilerClass extends React.Component {
constructor(props) {
super(props);
Expand Down Expand Up @@ -133,7 +63,7 @@ export class BugFilerClass extends React.Component {
summaryString = summaryString.replace(re, '');
}

const crashSignatures = this.getCrashSignatures(suggestion);
const crashSignatures = getCrashSignatures(suggestion);

const keywords = [];
let isAssertion = [
Expand Down Expand Up @@ -283,12 +213,6 @@ export class BugFilerClass extends React.Component {
this.findProductByPath();
}

getCrashSignatures(failureLine) {
const crashRegex = /(\[@ .+\])/g;
const crashSignatures = failureLine.search.match(crashRegex);
return crashSignatures ? [crashSignatures[0]] : [];
}

getUnhelpfulSummaryReason(summary) {
const { suggestion } = this.props;
const searchTerms = suggestion.search_terms;
Expand Down Expand Up @@ -681,7 +605,7 @@ export class BugFilerClass extends React.Component {
selectedProduct,
} = this.state;
const searchTerms = suggestion.search_terms;
const crashSignatures = this.getCrashSignatures(suggestion);
const crashSignatures = getCrashSignatures(suggestion);
const unhelpfulSummaryReason = this.getUnhelpfulSummaryReason(summary);

return (
Expand Down
Loading