Commit 88f8ca14 authored by Karandeep Bhatia's avatar Karandeep Bhatia Committed by Commit Bot

DocServer: Fix failing presubmit and broken cron.

Since r786851, the docserver integration test is hanging causing
presubmits to fail. Also, the cron script which we use to update the
docserver is hanging. This is because r786851 added dependencies on
behavior features. However behavior features are not accounted for in
features_bundles.py which is used to resolve feature characteristics
like channel, platform etc. Since the code in the file expects that all
features will eventually be resolved, it keeps running indefinitely.

To fix:

- Ensure behavior features are ignored for the purpose of evalauting
feature characteristics for the docserver.

- Break out of the loop in GetFeatures if no new features are resolved
after a top level iteration. This'll ensure that the script terminates
eventually. Also, log an error if any features are left unresolved.

Other fixes:

- Delete cloudPrintPrivate.html. It was causing presubmit to fail since
beginning r785624, cloudPrintPrivate is disabled for extensions.

- Disable pylint checking in
chrome/common/extensions/docs/server2/PRESUBMIT.py. There are multiple
failures there already and it is causing the presubmit step to timeout.

BUG=1103846, 1105211
TEST=python
chrome/common/extensions/docs/server2/integration_test.py --
apps/developerPrivate doesn't hang

Change-Id: I640ea35a66ecd863e39ca7b9b2e3e5469bfae2a8
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2296866Reviewed-by: default avatarDevlin <rdevlin.cronin@chromium.org>
Commit-Queue: Karan Bhatia <karandeepb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#788844}
parent 0257de0a
...@@ -58,13 +58,7 @@ No? I guess this presubmit check doesn't work. ...@@ -58,13 +58,7 @@ No? I guess this presubmit check doesn't work.
def _RunPresubmit(input_api, output_api): def _RunPresubmit(input_api, output_api):
_BuildServer(input_api) _BuildServer(input_api)
# For now, print any lint errors. When these have been eliminated, # TODO(crbug.com/434363): Enable pylint for the docserver.
# move these into the warning list below.
# See crbug.com/434363 and crbug.com/461130.
lint_errors = input_api.canned_checks.RunPylint(input_api, output_api)
if lint_errors:
print(lint_errors)
return ( return (
_WarnIfAppYamlHasntChanged(input_api, output_api) + _WarnIfAppYamlHasntChanged(input_api, output_api) +
input_api.canned_checks.RunUnitTestsInDirectory( input_api.canned_checks.RunUnitTestsInDirectory(
......
application: chrome-apps-doc application: chrome-apps-doc
version: 3-69-0 version: 3-70-0
runtime: python27 runtime: python27
api_version: 1 api_version: 1
threadsafe: false threadsafe: false
......
...@@ -2,10 +2,11 @@ ...@@ -2,10 +2,11 @@
# Use of this source code is governed by a BSD-style license that can be # Use of this source code is governed by a BSD-style license that can be
# found in the LICENSE file. # found in the LICENSE file.
from copy import copy import logging
from branch_utility import BranchUtility from branch_utility import BranchUtility
from compiled_file_system import SingleFile, Unicode from compiled_file_system import SingleFile, Unicode
from copy import copy
from docs_server_utils import StringIdentity from docs_server_utils import StringIdentity
from extensions_paths import API_PATHS, JSON_TEMPLATES from extensions_paths import API_PATHS, JSON_TEMPLATES
from file_system import FileNotFoundError from file_system import FileNotFoundError
...@@ -135,6 +136,13 @@ def _ResolveFeature(feature_name, ...@@ -135,6 +136,13 @@ def _ResolveFeature(feature_name,
for dependency in dependencies: for dependency in dependencies:
dep_type, dep_name = dependency.split(':') dep_type, dep_name = dependency.split(':')
# Ignore dependencies on behavior features for the purpose of resolving
# feature characteristics, since behavior features are generally used for
# configuring certain specific allowlisted extension behaviors.
if dep_type == 'behavior':
continue
if (dep_type not in features_map or if (dep_type not in features_map or
dep_name in features_map[dep_type].get('unresolved', ())): dep_name in features_map[dep_type].get('unresolved', ())):
# The dependency itself has not been merged yet or the features map # The dependency itself has not been merged yet or the features map
...@@ -345,10 +353,23 @@ class FeaturesBundle(object): ...@@ -345,10 +353,23 @@ class FeaturesBundle(object):
return any(cache.get('unresolved') return any(cache.get('unresolved')
for cache in features_map.itervalues()) for cache in features_map.itervalues())
# Iterate until everything is resolved. If dependencies are multiple def get_unresolved():
# levels deep, it might take multiple passes to inherit data to the '''Returns a dictionary of unresolved features mapping the type of
# topmost feature. features to the list of unresolved feature names
while has_unresolved(): '''
unresolved = {}
for cache_type, cache in features_map.iteritems():
if 'unresolved' not in cache:
continue
unresolved[cache_type] = cache['unresolved'].keys()
return unresolved
# Iterate until we can't resolve any more features. If dependencies
# are multiple levels deep, it might take multiple passes to inherit
# data to the topmost feature.
any_resolved = True
while has_unresolved() and any_resolved:
any_resolved = False
for cache_type, cache in features_map.iteritems(): for cache_type, cache in features_map.iteritems():
if 'unresolved' not in cache: if 'unresolved' not in cache:
continue continue
...@@ -364,6 +385,9 @@ class FeaturesBundle(object): ...@@ -364,6 +385,9 @@ class FeaturesBundle(object):
if not resolve_successful: if not resolve_successful:
continue # Try again on the next iteration of the while loop continue # Try again on the next iteration of the while loop
if resolve_successful:
any_resolved = True
# When successfully resolved, remove it from the unresolved # When successfully resolved, remove it from the unresolved
# dict. Add it to the resolved dict if it didn't get deleted. # dict. Add it to the resolved dict if it didn't get deleted.
to_remove.append(name) to_remove.append(name)
...@@ -373,6 +397,12 @@ class FeaturesBundle(object): ...@@ -373,6 +397,12 @@ class FeaturesBundle(object):
for key in to_remove: for key in to_remove:
del cache['unresolved'][key] del cache['unresolved'][key]
# TODO(karandeepb): Add a test to ensure that all features are
# correctly resolved.
if has_unresolved():
logging.error('Some features were left unresolved ' +
str(get_unresolved()))
for cache_type, cache in features_map.iteritems(): for cache_type, cache in features_map.iteritems():
self._object_store.Set(cache_type, cache['resolved']) self._object_store.Set(cache_type, cache['resolved'])
return features_map[features_type]['resolved'] return features_map[features_type]['resolved']
......
{{+partials.standard_apps_api api:apis.extensions.cloudPrintPrivate/}}
...@@ -4,6 +4,8 @@ ...@@ -4,6 +4,8 @@
// This features file defines switches used to control Extension behaviour, // This features file defines switches used to control Extension behaviour,
// typically whitelist configuration. // typically whitelist configuration.
// Note, the features defined here are generally ignored for the purposes of
// extension documentation.
// See chrome/common/extensions/api/_features.md to understand this file, as // See chrome/common/extensions/api/_features.md to understand this file, as
// well as feature.h, simple_feature.h, and feature_provider.h. // well as feature.h, simple_feature.h, and feature_provider.h.
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment