[Notes] [Git][BuildStream/buildstream][bschubert/tests-no-chroot-linux] 4 commits: element.py: Use _force_rmtree instead of custom code.

[Benjamin Schubert <gitlab mg gitlab com>]

Title: GitLab

Benjamin Schubert pushed to branch bschubert/tests-no-chroot-linux at BuildStream / buildstream

Commits:

• 261e2cd3
  by Jim MacArthur at 2018-11-07T12:02:43Z

  element.py: Use _force_rmtree instead of custom code.
  
  There are arguments for and against using onerror - see issue #735
  for details.
  

• 8931e42c
  by Jim MacArthur at 2018-11-07T12:26:06Z

  Merge branch 'jmac/make_writable_combination' into 'master'
  
  Use _force_rmtree instead of custom code.
  
  Closes #735
  
  See merge request BuildStream/buildstream!927

• 81ea9688
  by Benjamin Schubert at 2018-11-07T12:46:43Z

  Ensure platform is not maintained between tests
  
  This removes the `_instance` on the platform object that we use
  for caching and not recreating the object everytime at the start
  of every test.
  
  This is to ensure our tests share the least amount of state.
  
  The performance penalty is from 2 to 5% accross the whole test suite.
  The readings were done 5 times for each before and after the change
  and on the same computer.
  

• f549d812
  by Benjamin Schubert at 2018-11-07T12:46:43Z

  Ensure helpful messages are raised when missing dependencies
  
  We need to supress the brwap version caching for it to work.
  
  This caching leaks for every test but removing it every time seems
  too prohibitive, given that, normally, the bwrap version would not
  change.
  
  On a fedora 29 laptop, the performance degradation was around 5-10%
  over the whole test suite if used globally.
  

4 changed files:

• buildstream/element.py
• buildstream/sandbox/_sandboxdummy.py
• conftest.py
• + tests/integration/missing_dependencies.py

Changes:

buildstream/element.py

---

...	...	@@ -1410,16 +1410,9 @@ class Element(Plugin):
1410	1410
1411	1411	finally:
1412	1412	# Staging may produce directories with less than 'rwx' permissions
1413		- # for the owner, which will break tempfile, so we need to use chmod
1414		- # occasionally.
1415		- def make_dir_writable(fn, path, excinfo):
1416		- os.chmod(os.path.dirname(path), 0o777)
1417		- if os.path.isdir(path):
1418		- os.rmdir(path)
1419		- else:
1420		- os.remove(path)
1421		- shutil.rmtree(temp_staging_directory, onerror=make_dir_writable)
1422		-
	1413	+ # for the owner, which breaks tempfile. _force_rmtree will deal
	1414	+ # with these.
	1415	+ utils._force_rmtree(temp_staging_directory)
1423	1416	# Ensure deterministic mtime of sources at build time
1424	1417	vdirectory.set_deterministic_mtime()
1425	1418	# Ensure deterministic owners of sources at build time

buildstream/sandbox/_sandboxdummy.py

---

...	...	@@ -42,4 +42,5 @@ class SandboxDummy(Sandbox):
42	42	"'{}'".format(command[0]),
43	43	reason='missing-command')
44	44
45		- raise SandboxError("This platform does not support local builds: {}".format(self._reason))
	45	+ raise SandboxError("This platform does not support local builds: {}".format(self._reason),
	46	+ reason="unavailable-local-sandbox")

conftest.py

---

...	...	@@ -23,6 +23,8 @@ import shutil
23	23
24	24	import pytest
25	25
	26	+from buildstream._platform.platform import Platform
	27	+
26	28
27	29	def pytest_addoption(parser):
28	30	parser.addoption('--integration', action='store_true', default=False,
...	...	@@ -52,3 +54,8 @@ def integration_cache(request):
52	54	shutil.rmtree(os.path.join(cache_dir, 'artifacts'))
53	55	except FileNotFoundError:
54	56	pass
	57	+
	58	+
	59	+@pytest.fixture(autouse=True)
	60	+def clean_platform_cache():
	61	+ Platform._instance = None

tests/integration/missing_dependencies.py

---

	1	+import os
	2	+import pytest
	3	+from tests.testutils import cli
	4	+from tests.testutils.site import IS_LINUX
	5	+
	6	+from buildstream import _site, _yaml
	7	+from buildstream._exceptions import ErrorDomain
	8	+
	9	+
	10	+pytestmark = pytest.mark.integration
	11	+
	12	+
	13	+# Project directory
	14	+DATA_DIR = os.path.join(
	15	+ os.path.dirname(os.path.realpath(__file__)),
	16	+ 'project',
	17	+)
	18	+
	19	+
	20	+# clean_bwrap_cache()
	21	+#
	22	+# Removes the cached information about bwrap in _site.
	23	+#
	24	+# This removes them before the test so the test is clean and after the test
	25	+# so we don't have side effects on the next tests.
	26	+#
	27	+# Ideally this would be run before each test but that is slowing down
	28	+# the tests.
	29	+@pytest.fixture
	30	+def clean_bwrap_cache():
	31	+ def wipe():
	32	+ _site._bwrap_major = None
	33	+ _site._bwrap_minor = None
	34	+ _site._bwrap_patch = None
	35	+
	36	+ wipe()
	37	+ yield
	38	+ wipe()
	39	+
	40	+
	41	+@pytest.mark.skipif(not IS_LINUX, reason='Only available on Linux')
	42	+@pytest.mark.datafiles(DATA_DIR)
	43	+@pytest.mark.usefixtures("clean_bwrap_cache")
	44	+def test_missing_brwap_has_nice_error_message(cli, datafiles):
	45	+ project = os.path.join(datafiles.dirname, datafiles.basename)
	46	+ element_path = os.path.join(project, 'elements', 'element.bst')
	47	+
	48	+ # Write out our test target
	49	+ element = {
	50	+ 'kind': 'script',
	51	+ 'depends': [
	52	+ {
	53	+ 'filename': 'base.bst',
	54	+ 'type': 'build',
	55	+ },
	56	+ ],
	57	+ 'config': {
	58	+ 'commands': [
	59	+ 'false',
	60	+ ],
	61	+ },
	62	+ }
	63	+ _yaml.dump(element, element_path)
	64	+
	65	+ # Build without access to host tools, this should fail with a nice error
	66	+ result = cli.run(
	67	+ project=project, args=['build', 'element.bst'], env={'PATH': ''})
	68	+ result.assert_task_error(ErrorDomain.SANDBOX, 'unavailable-local-sandbox')
	69	+ assert "not found" in result.stderr
	70	+
	71	+
	72	+@pytest.mark.skipif(not IS_LINUX, reason='Only available on Linux')
	73	+@pytest.mark.datafiles(DATA_DIR)
	74	+@pytest.mark.usefixtures("clean_bwrap_cache")
	75	+def test_old_brwap_has_nice_error_message(cli, datafiles, tmp_path):
	76	+ import buildstream._platform
	77	+ buildstream._platform.Platform._instance = None
	78	+
	79	+ bwrap = tmp_path.joinpath('bin/bwrap')
	80	+ bwrap.parent.mkdir()
	81	+ with bwrap.open('w') as fp:
	82	+ fp.write('''
	83	+ #!/bin/bash
	84	+ echo bubblewrap 0.0.1
	85	+ '''.strip())
	86	+
	87	+ bwrap.chmod(0o755)
	88	+
	89	+ project = os.path.join(datafiles.dirname, datafiles.basename)
	90	+ element_path = os.path.join(project, 'elements', 'element3.bst')
	91	+
	92	+ # Write out our test target
	93	+ element = {
	94	+ 'kind': 'script',
	95	+ 'depends': [
	96	+ {
	97	+ 'filename': 'base.bst',
	98	+ 'type': 'build',
	99	+ },
	100	+ ],
	101	+ 'config': {
	102	+ 'commands': [
	103	+ 'true && true && false',
	104	+ ],
	105	+ },
	106	+ }
	107	+ _yaml.dump(element, element_path)
	108	+
	109	+ # Build without access to host tools, this should fail with a nice error
	110	+ result = cli.run(
	111	+ project=project,
	112	+ args=['--debug', '--verbose', 'build', 'element3.bst'],
	113	+ env={'PATH': str(tmp_path.joinpath('bin'))})
	114	+ result.assert_task_error(ErrorDomain.SANDBOX, 'unavailable-local-sandbox')
	115	+ assert "too old" in result.stderr

—
View it on GitLab.
You're receiving this email because of your account on gitlab.com. If you'd like to receive fewer emails, you can adjust your notification settings.