Skip to content
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

Interrupting wptrunner as manifest is regenerated causes MANIFEST.json to be corrupted #7142

Closed
wpt-issue-mover opened this issue Aug 31, 2017 · 4 comments
Labels
infra priority:backlog wptrunner The automated test runner, commonly called through ./wpt run

Comments

@wpt-issue-mover
Copy link

Originally posted as w3c/wptrunner#256 by @andreastt on 31 Jul 2017, 13:51 UTC:

I’m unable to reproduce this exactly, but interrupting wptrunner --manifest-update is dangerous because the MANIFEST.json it regenerates could be corrupted. This will lead to errors such as this the next time wptrunner is invoked:

% ./mach wpt --manifest-update
 0:00.23 LOG: MainThread INFO Updating test manifest /home/ato/src/gecko/testing/web-platform/meta/MANIFEST.json
 0:00.67 LOG: MainThread INFO Closing logging queue
 0:00.67 LOG: MainThread INFO queue closed
Error running mach:

    ['wpt', '--manifest-update']

The error occurred in code that was called by the mach command. This is either
a bug in the called code itself or in the way that mach is calling it.

You should consider filing a bug for this issue.

If filing a bug, please include the full output of mach, including this error
message.

The details of the failure are as follows:

ValueError: No JSON object could be decoded

  File "/home/ato/src/gecko/testing/web-platform/mach_commands.py", line 364, in run_wpt
    return self.run_web_platform_tests(**params)
  File "/home/ato/src/gecko/testing/web-platform/mach_commands.py", line 357, in run_web_platform_tests
    return wpt_runner.run(**params)
  File "/home/ato/src/gecko/testing/web-platform/mach_commands_base.py", line 28, in run
    result = wptrunner.start(**kwargs)
  File "/home/ato/src/gecko/testing/web-platform/tests/tools/wptrunner/wptrunner/wptrunner.py", line 265, in start
    return not run_tests(**kwargs)
  File "/home/ato/src/gecko/testing/web-platform/tests/tools/wptrunner/wptrunner/wptrunner.py", line 153, in run_tests
    **kwargs)
  File "/home/ato/src/gecko/testing/web-platform/tests/tools/wptrunner/wptrunner/wptrunner.py", line 47, in get_loader
    test_manifests = testloader.ManifestLoader(test_paths, force_manifest_update=kwargs["manifest_update"]).load()
  File "/home/ato/src/gecko/testing/web-platform/tests/tools/wptrunner/wptrunner/testloader.py", line 386, in load
    **paths)
  File "/home/ato/src/gecko/testing/web-platform/tests/tools/wptrunner/wptrunner/testloader.py", line 424, in load_manifest
    self.update_manifest(manifest_path, tests_path, url_base)
  File "/home/ato/src/gecko/testing/web-platform/tests/tools/wptrunner/wptrunner/testloader.py", line 403, in update_manifest
    json_data = json.load(f)
  File "/usr/lib/python2.7/json/__init__.py", line 291, in load
    **kw)
  File "/usr/lib/python2.7/json/__init__.py", line 339, in loads
    return _default_decoder.decode(s)
  File "/usr/lib/python2.7/json/decoder.py", line 364, in decode
    obj, end = self.raw_decode(s, idx=_w(s, 0).end())
  File "/usr/lib/python2.7/json/decoder.py", line 382, in raw_decode
    raise ValueError("No JSON object could be decoded")
@wpt-issue-mover
Copy link
Author

Originally posted as w3c/wptrunner#256 (comment) by @gsnedders on 01 Aug 2017, 14:42 UTC:

This should probably be reported against the wpt repo since it was monorepo'd.

@gsnedders gsnedders added infra wptrunner The automated test runner, commonly called through ./wpt run labels Aug 31, 2017
vedantc98 added a commit to vedantc98/web-platform-tests that referenced this issue Feb 17, 2018
Earlier, interrupting the manifest-update could end up
causing errors due to the non-atomicity of the process.
Used tempfile to ensure that the file is written correctly.
vedantc98 added a commit to vedantc98/web-platform-tests that referenced this issue Feb 17, 2018
Earlier, interrupting the manifest-update could end up
causing errors due to the non-atomicity of the process.
Used tempfile to ensure that the file is written correctly.
vedantc98 added a commit to vedantc98/web-platform-tests that referenced this issue Feb 17, 2018
Earlier, interrupting the manifest-update could end up
causing errors due to the non-atomicity of the process.
Used tempfile to ensure that the file is written correctly.
vedantc98 added a commit to vedantc98/web-platform-tests that referenced this issue Feb 17, 2018
Earlier, interrupting the manifest-update could end up
causing errors due to the non-atomicity of the process.
Used tempfile to ensure that the file is written correctly.
vedantc98 added a commit to vedantc98/web-platform-tests that referenced this issue Feb 18, 2018
Earlier, interrupting the manifest-update could end up
causing errors due to the non-atomicity of the process.
Used tempfile to ensure that the file is written correctly.
vedantc98 added a commit to vedantc98/web-platform-tests that referenced this issue Feb 18, 2018
Earlier, interrupting the manifest-update could end up
causing errors due to the non-atomicity of the process.
Used tempfile to ensure that the file is written correctly.
Hexcles added a commit that referenced this issue Mar 1, 2018
@Hexcles
Copy link
Member

Hexcles commented Mar 1, 2018

Reopening the issue because the fix caused a bug and was reverted.

@Hexcles Hexcles reopened this Mar 1, 2018
@Hexcles Hexcles closed this as completed in 5122353 Mar 1, 2018
@Hexcles Hexcles reopened this Mar 1, 2018
@saschanaz
Copy link
Member

Looking at #9736, there is a way to open a file while still allowing replacement: https://stackoverflow.com/a/18169890

win32file is already used by one of the dependency, so maybe can be used here?

@foolip
Copy link
Member

foolip commented May 6, 2021

This was fixed by #24032.

@foolip foolip closed this as completed May 6, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
infra priority:backlog wptrunner The automated test runner, commonly called through ./wpt run
Projects
None yet
Development

No branches or pull requests

6 participants