mirror of https://github.com/zulip/zulip.git
node tests: Use EXEMPT_FILES for line coverage.
We now have 100% line coverage on 71 JS files. This is thanks to about 150 people who have contributed code to frontend/node_tests. And then 126 files are still short of 100% line coverage. We now enforce line coverage with a set called EXEMPT_FILES, which are the files for which we do NOT expect to have 100% coverage. Using an exemption list makes it so that adding a new JS file to the project without 100% line coverage will cause the build to fail. This will encourage folks to be intentional about their lack of test coverage.
This commit is contained in:
Steve Howell
Tim Abbott
parent
fe48ede9cf
commit
e1977f4680
|
|
@ -1,5 +1,6 @@
|
||||||
#!/usr/bin/env python3
|
#!/usr/bin/env python3
|
||||||
import argparse
|
import argparse
|
||||||
|
import glob
|
||||||
import os
|
import os
|
||||||
import subprocess
|
import subprocess
|
||||||
import sys
|
import sys
|
||||||
|
|
@ -25,82 +26,134 @@ USAGE = '''
|
||||||
tools/test-js-with-node --coverage - to generate coverage report
|
tools/test-js-with-node --coverage - to generate coverage report
|
||||||
'''
|
'''
|
||||||
|
|
||||||
enforce_fully_covered = {
|
# We do not yet require 100% line coverage for these files:
|
||||||
'static/shared/js/typeahead.js',
|
EXEMPT_FILES = {
|
||||||
'static/shared/js/typing_status.js',
|
'static/js/admin.js',
|
||||||
|
'static/js/archive.js',
|
||||||
'static/js/activity.js',
|
'static/js/attachments_ui.js',
|
||||||
'static/js/alert_words.js',
|
'static/js/avatar.js',
|
||||||
'static/js/alert_words_ui.js',
|
'static/js/billing/helpers.js',
|
||||||
'static/js/bot_data.js',
|
'static/js/blueslip.js',
|
||||||
'static/js/buddy_data.js',
|
'static/js/blueslip_stacktrace.ts',
|
||||||
'static/js/buddy_list.js',
|
'static/js/click_handlers.js',
|
||||||
'static/js/channel.js',
|
'static/js/compose_actions.js',
|
||||||
'static/js/color_data.js',
|
'static/js/composebox_typeahead.js',
|
||||||
'static/js/colorspace.js',
|
'static/js/compose_fade.js',
|
||||||
'static/js/common.js',
|
'static/js/compose.js',
|
||||||
'static/js/components.js',
|
'static/js/condense.js',
|
||||||
'static/js/compose_pm_pill.js',
|
'static/js/confirm_dialog.js',
|
||||||
'static/js/compose_state.js',
|
'static/js/copy_and_paste.js',
|
||||||
'static/js/compose_ui.js',
|
'static/js/csrf.js',
|
||||||
# Temporarily removed until we finish merging emoji commits.
|
'static/js/debug.js',
|
||||||
# 'static/js/composebox_typeahead.js',
|
'static/js/drafts.js',
|
||||||
'static/js/dict.ts',
|
'static/js/echo.js',
|
||||||
'static/js/emoji.js',
|
'static/js/emoji_picker.js',
|
||||||
'static/js/feature_flags.js',
|
'static/js/favicon.js',
|
||||||
'static/js/fenced_code.js',
|
'static/js/feedback_widget.js',
|
||||||
'static/js/fetch_status.js',
|
'static/js/floating_recipient_bar.js',
|
||||||
'static/js/filter.js',
|
'static/js/gear_menu.js',
|
||||||
'static/js/fold_dict.ts',
|
'static/js/global.d.ts',
|
||||||
'static/js/hash_util.js',
|
'static/js/hashchange.js',
|
||||||
'static/js/keydown_util.js',
|
'static/js/hbs.d.ts',
|
||||||
'static/js/input_pill.js',
|
'static/js/hotkey.js',
|
||||||
'static/js/int_dict.ts',
|
'static/js/hotspots.js',
|
||||||
'static/js/lazy_set.js',
|
'static/js/info_overlay.js',
|
||||||
'static/js/list_cursor.js',
|
'static/js/integration_bot_widget.js',
|
||||||
'static/js/markdown.js',
|
'static/js/invite.js',
|
||||||
'static/js/message_store.js',
|
'static/js/lightbox_canvas.js',
|
||||||
'static/js/muting.js',
|
'static/js/lightbox.js',
|
||||||
'static/js/narrow_state.js',
|
'static/js/list_render.js',
|
||||||
'static/js/people.js',
|
'static/js/list_util.js',
|
||||||
'static/js/pm_conversations.js',
|
'static/js/loading.js',
|
||||||
'static/js/pm_list.js',
|
'static/js/local_message.js',
|
||||||
'static/js/presence.js',
|
'static/js/localstorage.js',
|
||||||
'static/js/reactions.js',
|
'static/js/message_edit.js',
|
||||||
'static/js/recent_senders.js',
|
'static/js/message_events.js',
|
||||||
'static/js/rtl.js',
|
'static/js/message_fetch.js',
|
||||||
'static/js/schema.js',
|
'static/js/message_flags.js',
|
||||||
'static/js/scroll_util.js',
|
'static/js/message_list_data.js',
|
||||||
'static/js/search.js',
|
'static/js/message_list.js',
|
||||||
'static/js/search_suggestion.js',
|
'static/js/message_list_view.js',
|
||||||
'static/js/search_util.js',
|
'static/js/message_live_update.js',
|
||||||
'static/js/server_events_dispatch.js',
|
'static/js/message_scroll.js',
|
||||||
# Removed because we're migrating code from uncovered other settings pages to here.
|
'static/js/message_util.js',
|
||||||
# 'static/js/settings_ui.js',
|
'static/js/message_viewport.js',
|
||||||
'static/js/settings_muting.js',
|
'static/js/muting_ui.js',
|
||||||
'static/js/settings_user_groups.js',
|
'static/js/narrow.js',
|
||||||
'static/js/stream_data.js',
|
'static/js/navigate.js',
|
||||||
'static/js/stream_events.js',
|
'static/js/night_mode.js',
|
||||||
'static/js/stream_sort.js',
|
'static/js/notifications.js',
|
||||||
'static/js/top_left_corner.js',
|
'static/js/overlays.js',
|
||||||
'static/js/topic_data.js',
|
'static/js/padded_widget.js',
|
||||||
'static/js/topic_list_data.js',
|
'static/js/page_params.js',
|
||||||
'static/js/topic_generator.js',
|
'static/js/panels.js',
|
||||||
'static/js/transmit.js',
|
'static/js/pm_list_dom.js',
|
||||||
'static/js/typeahead_helper.js',
|
'static/js/pointer.js',
|
||||||
'static/js/typing_data.js',
|
'static/js/poll_widget.js',
|
||||||
'static/js/unread.js',
|
'static/js/popovers.js',
|
||||||
'static/js/user_events.js',
|
'static/js/ready.js',
|
||||||
'static/js/user_groups.js',
|
'static/js/realm_icon.js',
|
||||||
'static/js/user_pill.js',
|
'static/js/realm_logo.js',
|
||||||
'static/js/user_search.js',
|
'static/js/reload.js',
|
||||||
'static/js/user_status.js',
|
'static/js/reload_state.js',
|
||||||
'static/js/util.js',
|
'static/js/reminder.js',
|
||||||
'static/js/vdom.js',
|
'static/js/resize.js',
|
||||||
'static/js/widgetize.js',
|
'static/js/rows.js',
|
||||||
'static/js/search_pill.js',
|
'static/js/scroll_bar.js',
|
||||||
'static/js/billing/billing.js',
|
'static/js/search_pill_widget.js',
|
||||||
'static/js/billing/upgrade.js',
|
'static/js/sent_messages.js',
|
||||||
|
'static/js/server_events.js',
|
||||||
|
'static/js/settings_account.js',
|
||||||
|
'static/js/settings_bots.js',
|
||||||
|
'static/js/settings_display.js',
|
||||||
|
'static/js/settings_emoji.js',
|
||||||
|
'static/js/settings_exports.js',
|
||||||
|
'static/js/settings_invites.js',
|
||||||
|
'static/js/settings.js',
|
||||||
|
'static/js/settings_linkifiers.js',
|
||||||
|
'static/js/settings_notifications.js',
|
||||||
|
'static/js/settings_org.js',
|
||||||
|
'static/js/settings_panel_menu.js',
|
||||||
|
'static/js/settings_profile_fields.js',
|
||||||
|
'static/js/settings_sections.js',
|
||||||
|
'static/js/settings_streams.js',
|
||||||
|
'static/js/settings_toggle.js',
|
||||||
|
'static/js/settings_ui.js',
|
||||||
|
'static/js/settings_users.js',
|
||||||
|
'static/js/setup.js',
|
||||||
|
'static/js/starred_messages.js',
|
||||||
|
'static/js/stream_color.js',
|
||||||
|
'static/js/stream_create.js',
|
||||||
|
'static/js/stream_edit.js',
|
||||||
|
'static/js/stream_list.js',
|
||||||
|
'static/js/stream_muting.js',
|
||||||
|
'static/js/stream_popover.js',
|
||||||
|
'static/js/stream_ui_updates.js',
|
||||||
|
'static/js/submessage.js',
|
||||||
|
'static/js/subs.js',
|
||||||
|
'static/js/tab_bar.js',
|
||||||
|
'static/js/templates.js',
|
||||||
|
'static/js/tictactoe_widget.js',
|
||||||
|
'static/js/timerender.js',
|
||||||
|
'static/js/todo_widget.js',
|
||||||
|
'static/js/topic_list.js',
|
||||||
|
'static/js/topic_zoom.js',
|
||||||
|
'static/js/translations.js',
|
||||||
|
'static/js/tutorial.js',
|
||||||
|
'static/js/typing_events.js',
|
||||||
|
'static/js/typing.js',
|
||||||
|
'static/js/ui_init.js',
|
||||||
|
'static/js/ui.js',
|
||||||
|
'static/js/ui_report.js',
|
||||||
|
'static/js/ui_util.js',
|
||||||
|
'static/js/unread_ops.js',
|
||||||
|
'static/js/unread_ui.js',
|
||||||
|
'static/js/upload.js',
|
||||||
|
'static/js/upload_widget.js',
|
||||||
|
'static/js/user_status_ui.js',
|
||||||
|
'static/js/zcommand.js',
|
||||||
|
'static/js/zform.js',
|
||||||
|
'static/js/zulip.js',
|
||||||
}
|
}
|
||||||
|
|
||||||
parser = argparse.ArgumentParser(USAGE)
|
parser = argparse.ArgumentParser(USAGE)
|
||||||
|
|
@ -191,6 +244,15 @@ def read_coverage() -> Any:
|
||||||
return coverage_json
|
return coverage_json
|
||||||
|
|
||||||
def enforce_proper_coverage(coverage_json: Any) -> bool:
|
def enforce_proper_coverage(coverage_json: Any) -> bool:
|
||||||
|
all_js_files = set(
|
||||||
|
glob.glob('static/js/*.js') +
|
||||||
|
glob.glob('static/js/*.ts') +
|
||||||
|
glob.glob('static/shared/js/*.js') +
|
||||||
|
glob.glob('static/shared/js/*.ts') +
|
||||||
|
glob.glob('static/js/billing/*.js')
|
||||||
|
)
|
||||||
|
enforce_fully_covered = all_js_files - EXEMPT_FILES
|
||||||
|
|
||||||
coverage_lost = False
|
coverage_lost = False
|
||||||
for relative_path in enforce_fully_covered:
|
for relative_path in enforce_fully_covered:
|
||||||
path = ROOT_DIR + "/" + relative_path
|
path = ROOT_DIR + "/" + relative_path
|
||||||
|
|
@ -205,25 +267,25 @@ def enforce_proper_coverage(coverage_json: Any) -> bool:
|
||||||
if coverage_lost:
|
if coverage_lost:
|
||||||
print()
|
print()
|
||||||
print("It looks like your changes lost 100% test coverage in one or more files.")
|
print("It looks like your changes lost 100% test coverage in one or more files.")
|
||||||
print("Usually, the right fix for this is to add some tests.")
|
print("Ideally, you should add some tests to restore coverage.")
|
||||||
print("But also check out the include/exclude lists in `tools/test-js-with-node`.")
|
print("A worse option is to update EXEMPT_FILES in `tools/test-js-with-node`.")
|
||||||
print("To run this check locally, use `test-js-with-node --coverage`.")
|
print("To run this check locally, use `test-js-with-node --coverage`.")
|
||||||
|
print()
|
||||||
|
|
||||||
coverage_not_enforced = False
|
coverage_not_enforced = False
|
||||||
for path in coverage_json:
|
for path in coverage_json:
|
||||||
if '/static/js/' in path or '/static/shared/js' in path:
|
|
||||||
relative_path = os.path.relpath(path, ROOT_DIR)
|
relative_path = os.path.relpath(path, ROOT_DIR)
|
||||||
|
if relative_path in EXEMPT_FILES:
|
||||||
line_coverage = coverage_json[path]['s']
|
line_coverage = coverage_json[path]['s']
|
||||||
line_mapping = coverage_json[path]['statementMap']
|
line_mapping = coverage_json[path]['statementMap']
|
||||||
if check_line_coverage(relative_path, line_coverage, line_mapping, log=False) \
|
if check_line_coverage(relative_path, line_coverage, line_mapping, log=False):
|
||||||
and not (relative_path in enforce_fully_covered):
|
|
||||||
coverage_not_enforced = True
|
coverage_not_enforced = True
|
||||||
print("ERROR: %s has complete node test coverage and is not enforced." % (relative_path,))
|
print("ERROR: {} unexpectedly has 100% line coverage.".format(relative_path))
|
||||||
|
|
||||||
if coverage_not_enforced:
|
if coverage_not_enforced:
|
||||||
print()
|
print()
|
||||||
print("There are one or more fully covered files that are not enforced.")
|
print("One or more fully covered files are miscategorized.")
|
||||||
print("Add the file(s) to enforce_fully_covered in `tools/test-js-with-node`.")
|
print("Remove the file(s) from EXEMPT_FILES in `tools/test-js-with-node`.")
|
||||||
|
|
||||||
problems_encountered = (coverage_lost or coverage_not_enforced)
|
problems_encountered = (coverage_lost or coverage_not_enforced)
|
||||||
return problems_encountered
|
return problems_encountered
|
||||||
|
|
|
||||||
Loading…
Reference in New Issue