Skip to content

Commit

Permalink
Fix python deprecation warnings (elastic#20603)
Browse files Browse the repository at this point in the history
Solve python deprecation warnings, and make python tests to
fail if deprecated code is added.

Changes here:
* Add error::DeprecationWarning to pytest's filterwarnings (so tests
  fail if use deprecated code).
* Add pytest.ini to the list of files that trigger all CI builds.
* Refactor tests to don't require deprecated assertDictContainsSubset.
* Update autopep8 to latest version, and run it once with -a (aggresive).
* Solve some other deprecation warnings not solved automatically by
  autopep8 -a.
  • Loading branch information
jsoriano authored and melchiormoulin committed Oct 14, 2020
1 parent 27c5f5e commit ffd894d
Show file tree
Hide file tree
Showing 34 changed files with 87 additions and 85 deletions.
2 changes: 2 additions & 0 deletions Jenkinsfile
Original file line number Diff line number Diff line change
Expand Up @@ -1127,6 +1127,7 @@ def isChangedOSSCode(patterns) {
def allPatterns = [
"^Jenkinsfile",
"^go.mod",
"^pytest.ini",
"^libbeat/.*",
"^testing/.*",
"^dev-tools/.*",
Expand All @@ -1140,6 +1141,7 @@ def isChangedXPackCode(patterns) {
def allPatterns = [
"^Jenkinsfile",
"^go.mod",
"^pytest.ini",
"^libbeat/.*",
"^dev-tools/.*",
"^testing/.*",
Expand Down
2 changes: 1 addition & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -167,7 +167,7 @@ notice:
.PHONY: python-env
python-env:
@test -d $(PYTHON_ENV) || ${PYTHON_EXE} -m venv $(VENV_PARAMS) $(PYTHON_ENV)
@$(PYTHON_ENV)/bin/pip install -q --upgrade pip autopep8==1.3.5 pylint==2.4.4
@$(PYTHON_ENV)/bin/pip install -q --upgrade pip autopep8==1.5.4 pylint==2.4.4
@# Work around pip bug. See: https://github.com/pypa/pip/issues/4464
@find $(PYTHON_ENV) -type d -name dist-packages -exec sh -c "echo dist-packages > {}.pth" ';'

Expand Down
2 changes: 1 addition & 1 deletion auditbeat/tests/system/test_file_integrity.py
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ def file_events(objs, path, expected):
evts = set()
for obj in objs:
if 'file.path' in obj and 'event.action' in obj and obj['file.path'].lower() == path.lower():
if type(obj['event.action']) == list:
if isinstance(obj['event.action'], list):
evts = evts.union(set(obj['event.action']))
else:
evts.add(obj['event.action'])
Expand Down
2 changes: 1 addition & 1 deletion dev-tools/cmd/dashboards/export_5x_dashboards.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ def ExportDashboards(es, regex, kibana_index, output_directory):

try:
reg_exp = re.compile(regex, re.IGNORECASE)
except:
except BaseException:
print("Wrong regex {}".format(regex))
return

Expand Down
2 changes: 1 addition & 1 deletion filebeat/scripts/docs_collector.py
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ def collect(beat_name):
"""

# Write module docs
docs_path = os.path.join(os.path.abspath("docs"), "modules", module + ".asciidoc")
docs_path = os.path.join(os.path.abspath("docs"), "modules", module + ".asciidoc")
with open(docs_path, 'w', encoding='utf_8') as f:
f.write(module_file)

Expand Down
2 changes: 1 addition & 1 deletion filebeat/tests/open-file-handlers/log_stdout.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,5 +18,5 @@
# Setup python log handler
handler = logging.handlers.RotatingFileHandler(
log_file, maxBytes=line_length * lines_per_file + 1,
backupCount=int(total_lines/lines_per_file) + 1)
backupCount=int(total_lines / lines_per_file) + 1)
logger.addHandler(handler)
2 changes: 1 addition & 1 deletion filebeat/tests/system/filebeat.py
Original file line number Diff line number Diff line change
Expand Up @@ -176,7 +176,7 @@ def contains(self, msg, ignore_case=False, count=1):
if ignore_case:
msg = msg.lower()

if type(msg) == REGEXP_TYPE:
if isinstance(msg, REGEXP_TYPE):
def match(x): return msg.search(x) is not None
else:
def match(x): return x.find(msg) >= 0
Expand Down
2 changes: 1 addition & 1 deletion filebeat/tests/system/test_container.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ def test_container_input(self):

filebeat = self.start_beat()

self.wait_until(lambda: self.output_has(lines=21))
self.wait_until(lambda: self.output_has(lines=21))

filebeat.check_kill_and_wait()

Expand Down
2 changes: 1 addition & 1 deletion filebeat/tests/system/test_modules.py
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@ def run_on_file(self, module, fileset, test_file, cfgfile):

try:
self.es.indices.delete(index=self.index_name)
except:
except BaseException:
pass
self.wait_until(lambda: not self.es.indices.exists(self.index_name))

Expand Down
12 changes: 6 additions & 6 deletions filebeat/tests/system/test_multiline.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ def test_java_elasticsearch_log(self):
path=os.path.abspath(self.working_dir) + "/log/*",
multiline=True,
multiline_type="pattern",
pattern="^\[",
pattern=r"^\[",
negate="true",
match="after"
)
Expand Down Expand Up @@ -126,7 +126,7 @@ def test_max_lines(self):
path=os.path.abspath(self.working_dir) + "/log/*",
multiline=True,
multiline_type="pattern",
pattern="^\[",
pattern=r"^\[",
negate="true",
match="after",
max_lines=3
Expand Down Expand Up @@ -165,7 +165,7 @@ def test_timeout(self):
path=os.path.abspath(self.working_dir) + "/log/*",
multiline=True,
multiline_type="pattern",
pattern="^\[",
pattern=r"^\[",
negate="true",
match="after",
)
Expand Down Expand Up @@ -210,7 +210,7 @@ def test_max_bytes(self):
path=os.path.abspath(self.working_dir) + "/log/*",
multiline=True,
multiline_type="pattern",
pattern="^\[",
pattern=r"^\[",
negate="true",
match="after",
max_bytes=60
Expand Down Expand Up @@ -247,7 +247,7 @@ def test_close_timeout_with_multiline(self):
path=os.path.abspath(self.working_dir) + "/log/*",
multiline=True,
multiline_type="pattern",
pattern="^\[",
pattern=r"^\[",
negate="true",
match="after",
close_timeout="2s",
Expand Down Expand Up @@ -303,7 +303,7 @@ def test_consecutive_newline(self):
path=os.path.abspath(self.working_dir) + "/log/*",
multiline=True,
multiline_type="pattern",
pattern="^\[",
pattern=r"^\[",
negate="true",
match="after",
close_timeout="2s",
Expand Down
4 changes: 2 additions & 2 deletions filebeat/tests/system/test_pipeline.py
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ def test_input_pipeline_config(self):
index_name = "filebeat-test-input"
try:
self.es.indices.delete(index=index_name)
except:
except BaseException:
pass
self.wait_until(lambda: not self.es.indices.exists(index_name))

Expand Down Expand Up @@ -83,7 +83,7 @@ def search_objects():
res = self.es.search(index=index_name,
body={"query": {"match_all": {}}})
return [o["_source"] for o in res["hits"]["hits"]]
except:
except BaseException:
return []

self.wait_until(lambda: len(search_objects()) > 0, max_timeout=20)
Expand Down
33 changes: 13 additions & 20 deletions filebeat/tests/system/test_registrar.py
Original file line number Diff line number Diff line change
Expand Up @@ -61,10 +61,8 @@ def test_registrar_file_content(self):
logfile_abs_path = os.path.abspath(testfile_path)
record = self.get_registry_entry_by_path(logfile_abs_path)

self.assertDictContainsSubset({
"source": logfile_abs_path,
"offset": iterations * line_len,
}, record)
self.assertEqual(logfile_abs_path, record.get('source'))
self.assertEqual(iterations * line_len, record.get('offset'))
self.assertTrue("FileStateOS" in record)
self.assertTrue("meta" not in record)
file_state_os = record["FileStateOS"]
Expand All @@ -82,10 +80,8 @@ def test_registrar_file_content(self):
self.assertTrue("device" in file_state_os)
else:
stat = os.stat(logfile_abs_path)
self.assertDictContainsSubset({
"inode": stat.st_ino,
"device": stat.st_dev,
}, file_state_os)
self.assertEqual(stat.st_ino, file_state_os.get('inode'))
self.assertEqual(stat.st_dev, file_state_os.get('device'))

def test_registrar_files(self):
"""
Expand Down Expand Up @@ -347,7 +343,7 @@ def test_data_path(self):
self.wait_until(lambda: self.output_has(lines=1))
filebeat.check_kill_and_wait()

assert self.has_registry(data_path=self.working_dir+"/datapath")
assert self.has_registry(data_path=self.working_dir + "/datapath")

def test_rotating_file_inode(self):
"""
Expand Down Expand Up @@ -770,7 +766,8 @@ def test_state_after_rotation_ignore_older(self):
assert self.get_registry_entry_by_path(os.path.abspath(testfile_path1))["offset"] == 9
assert self.get_registry_entry_by_path(os.path.abspath(testfile_path2))["offset"] == 8

@unittest.skipIf(os.name == 'nt' or platform.system() == "Darwin", 'flaky test https://github.com/elastic/beats/issues/8102')
@unittest.skipIf(os.name == 'nt' or platform.system() == "Darwin",
'flaky test https://github.com/elastic/beats/issues/8102')
def test_clean_inactive(self):
"""
Checks that states are properly removed after clean_inactive
Expand Down Expand Up @@ -930,8 +927,8 @@ def test_restart_state(self):
ignore_older="2000ms",
)

init_files = ["test"+str(i)+".log" for i in range(3)]
restart_files = ["test"+str(i+3)+".log" for i in range(1)]
init_files = ["test" + str(i) + ".log" for i in range(3)]
restart_files = ["test" + str(i + 3) + ".log" for i in range(1)]

for name in init_files:
self.input_logs.write(name, "Hello World\n")
Expand Down Expand Up @@ -1318,10 +1315,8 @@ def test_registrar_files_with_input_level_processors(self):
logfile_abs_path = os.path.abspath(testfile_path1)
record = self.get_registry_entry_by_path(logfile_abs_path)

self.assertDictContainsSubset({
"source": logfile_abs_path,
"offset": iterations * (len("hello world") + len(os.linesep)),
}, record)
self.assertEqual(logfile_abs_path, record.get('source'))
self.assertEqual(iterations * (len("hello world") + len(os.linesep)), record.get('offset'))
self.assertTrue("FileStateOS" in record)
file_state_os = record["FileStateOS"]

Expand All @@ -1338,10 +1333,8 @@ def test_registrar_files_with_input_level_processors(self):
self.assertTrue("device" in file_state_os)
else:
stat = os.stat(logfile_abs_path)
self.assertDictContainsSubset({
"inode": stat.st_ino,
"device": stat.st_dev,
}, file_state_os)
self.assertEqual(stat.st_ino, file_state_os.get('inode'))
self.assertEqual(stat.st_dev, file_state_os.get('device'))

def test_registrar_meta(self):
"""
Expand Down
6 changes: 5 additions & 1 deletion filebeat/tests/system/test_setup.py
Original file line number Diff line number Diff line change
Expand Up @@ -60,4 +60,8 @@ def _setup_dummy_module(self):
os.mkdir(directory)

copytree(self.beat_path + "/tests/system/input/template-test-module", modules_path + "/template-test-module")
copyfile(self.beat_path + "/tests/system/input/template-test-module/_meta/config.yml", modules_d_path + "/test.yml")
copyfile(
self.beat_path +
"/tests/system/input/template-test-module/_meta/config.yml",
modules_d_path +
"/test.yml")
6 changes: 3 additions & 3 deletions heartbeat/scripts/generate_imports_helper.py
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
from os import listdir
from os.path import abspath, isdir, join

comment = """Package defaults imports all Monitor packages so that they
register with the global monitor registry. This package can be imported in the
main package to automatically register all of the standard supported Heartbeat
modules."""

from os.path import abspath, isdir, join
from os import listdir


blacklist = [
"monitors/active/dialchain"
Expand Down
2 changes: 1 addition & 1 deletion heartbeat/tests/system/test_base.py
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,7 @@ def run_fields(self, expected, local=None, top=None):
heartbeat_proc.check_kill_and_wait()

doc = self.read_output()[0]
self.assertDictContainsSubset(expected, doc)
assert expected.items() <= doc.items()
return doc

@unittest.skipUnless(INTEGRATION_TESTS, "integration test")
Expand Down
2 changes: 1 addition & 1 deletion heartbeat/tests/system/test_telemetry.py
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ def test_telemetry(self):
"Start job 'tcp-tcp@{}".format(tcp_host)))

init_lines = self.output_lines()
self.wait_until(lambda: self.output_has(lines=init_lines+2))
self.wait_until(lambda: self.output_has(lines=init_lines + 2))

self.assert_stats({
"http": {
Expand Down
4 changes: 2 additions & 2 deletions libbeat/scripts/generate_makefile_doc.py
Original file line number Diff line number Diff line change
Expand Up @@ -63,11 +63,11 @@ def parse_line(line, regexp, categories, categories_set):
try:
name = matches.group("varname")
is_variable = True
except:
except BaseException:
pass
try:
default = matches.group("default").strip()
except:
except BaseException:
default = ""

if not name:
Expand Down
14 changes: 7 additions & 7 deletions libbeat/tests/system/beat/beat.py
Original file line number Diff line number Diff line change
Expand Up @@ -109,12 +109,12 @@ def __del__(self):
try:
self.proc.terminate()
self.proc.kill()
except:
except BaseException:
pass
# Ensure the output is closed.
try:
self.output.close()
except:
except BaseException:
pass


Expand Down Expand Up @@ -180,7 +180,7 @@ def run_beat(self,
proc = self.start_beat(cmd=cmd, config=config, output=output,
logging_args=logging_args,
extra_args=extra_args, env=env)
if exit_code != None:
if exit_code is not None:
return proc.check_wait(exit_code)

return proc.wait()
Expand Down Expand Up @@ -276,7 +276,7 @@ def read_output(self,
try:
jsons.append(self.flatten_object(json.loads(
line, object_pairs_hook=self.json_raise_on_duplicates), []))
except:
except BaseException:
print("Fail to load the json {}".format(line))
raise

Expand Down Expand Up @@ -342,7 +342,7 @@ def setUp(self):
os.unlink(self.build_path + "last_run")
os.symlink(self.build_path + "run/{}".format(self.id()),
self.build_path + "last_run")
except:
except BaseException:
# symlink is best effort and can fail when
# running tests in parallel
pass
Expand Down Expand Up @@ -409,7 +409,7 @@ def log_contains_count(self, msg, logfile=None, ignore_case=False):
"""
Returns the number of appearances of the given string in the log file
"""
is_regexp = type(msg) == REGEXP_TYPE
is_regexp = isinstance(msg, REGEXP_TYPE)

counter = 0
if ignore_case:
Expand Down Expand Up @@ -755,7 +755,7 @@ def is_ecs_version_set(path):
# the file make that difficult
with open(path) as fhandle:
for line in fhandle:
if re.search("ecs\.version", line):
if re.search(r"ecs\.version", line):
return True
return False

Expand Down
12 changes: 6 additions & 6 deletions libbeat/tests/system/beat/common_tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -56,9 +56,9 @@ def test_export_index_pattern(self):
js = json.loads(output)
assert "objects" in js
size = len(output.encode('utf-8'))
assert size < 1024*1024, "Kibana index pattern must be less than 1MiB " \
"to keep the Beat setup request size below " \
"Kibana's server.maxPayloadBytes."
assert size < 1024 * 1024, "Kibana index pattern must be less than 1MiB " \
"to keep the Beat setup request size below " \
"Kibana's server.maxPayloadBytes."

def test_export_index_pattern_migration(self):
"""
Expand All @@ -68,9 +68,9 @@ def test_export_index_pattern_migration(self):
js = json.loads(output)
assert "objects" in js
size = len(output.encode('utf-8'))
assert size < 1024*1024, "Kibana index pattern must be less than 1MiB " \
"to keep the Beat setup request size below " \
"Kibana's server.maxPayloadBytes."
assert size < 1024 * 1024, "Kibana index pattern must be less than 1MiB " \
"to keep the Beat setup request size below " \
"Kibana's server.maxPayloadBytes."

def test_export_config(self):
"""
Expand Down
2 changes: 1 addition & 1 deletion libbeat/tests/system/beat/compose.py
Original file line number Diff line number Diff line change
Expand Up @@ -202,7 +202,7 @@ def compose_project_name(cls):
basename = os.path.basename(cls.find_compose_path())

def positivehash(x):
return hash(x) % ((sys.maxsize+1) * 2)
return hash(x) % ((sys.maxsize + 1) * 2)

return "%s_%X" % (basename, positivehash(frozenset(cls.COMPOSE_ENV.items())))

Expand Down
Loading

0 comments on commit ffd894d

Please sign in to comment.