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

Running tests doesn't work on Windows 10 64-bit #17392

Closed
camlorn opened this issue Jun 18, 2017 · 10 comments
Closed

Running tests doesn't work on Windows 10 64-bit #17392

camlorn opened this issue Jun 18, 2017 · 10 comments

Comments

@camlorn
Copy link
Contributor

@camlorn camlorn commented Jun 18, 2017

I get the following error for mach.bat test --all:

PS C:\projects\in_progress\servo> ./mach.bat test --all
Error running mach:

    ['test', '--all']

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:

TypeError: test_tidy() takes exactly 5 arguments (4 given)

  File "C:\projects\in_progress\servo\python\servo\testing_commands.py", line 156, in test
    Registrar.dispatch("test-%s" % suite, context=self.context, **kwargs)
  File "C:\projects\in_progress\servo\python\_virtualenv\Lib\site-packages\mach\registrar.py", line 123, in dispatch
    return self._run_command_handler(handler, context=context, **kwargs)
  File "C:\projects\in_progress\servo\python\_virtualenv\Lib\site-packages\mach\registrar.py", line 90, in _run_command_handler
    result = fn(**kwargs)
PS C:\projects\in_progress\servo>

And this from mach.bat test-wpt:

PS C:\projects\in_progress\servo> ./mach.bat test-wpt
Error running mach:

    ['test-wpt']

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:

TypeError: coercing to Unicode: need string or buffer, NoneType found

  File "C:\projects\in_progress\servo\python\servo\testing_commands.py", line 446, in test_wpt
    ret = self.run_test_list_or_dispatch(kwargs["test_list"], "wpt", self._test_wpt, **kwargs)
  File "C:\projects\in_progress\servo\python\servo\testing_commands.py", line 461, in run_test_list_or_dispatch
    return correct_function(**kwargs)
  File "C:\projects\in_progress\servo\python\servo\testing_commands.py", line 456, in _test_wpt
    return self.wptrunner(run_file, **kwargs)
  File "C:\projects\in_progress\servo\python\servo\testing_commands.py", line 502, in wptrunner
    return run_globals["run_tests"](**kwargs)
  File "C:\projects\in_progress\servo\tests\wpt\run_wpt.py", line 13, in run_tests
    return run.run_tests(paths=paths, **kwargs)
  File "C:\projects\in_progress\servo\tests\wpt\run.py", line 30, in run_tests
    set_defaults(paths, kwargs)
  File "C:\projects\in_progress\servo\tests\wpt\run.py", line 74, in set_defaults
    wptcommandline.check_args(kwargs)
  File "C:\projects\in_progress\servo\tests\wpt\harness\wptrunner\wptcommandline.py", line 328, in check_args
    elif exe_path(kwargs["openssl_binary"]) is not None:
  File "C:\projects\in_progress\servo\tests\wpt\harness\wptrunner\wptcommandline.py", line 259, in exe_path
    if os.access(path, os.X_OK):
PS C:\projects\in_progress\servo>

I've also tried these in a regular command shell. My default Python is a 64-bit 2.7 from Anaconda, but I've also tried it against a 32-bit regular install with identical results.

@camlorn
Copy link
Contributor Author

@camlorn camlorn commented Jun 19, 2017

Appveyor runs only test-unit. This works locally.

@camlorn
Copy link
Contributor Author

@camlorn camlorn commented Jun 19, 2017

I've looked into this some more. It's actually a bunch of small issues:

Some mach commands such as mach test fail because of Python errors. Others rely on bash scripts or Linux commands, which can't ever work on windows.

I was hoping it'd be an easy and quick fix.

@jdm
Copy link
Member

@jdm jdm commented Jun 19, 2017

#14039 means that @metajack got test-wpt working late last year. Apparently something regressed since then, but maybe there is useful inspiration to be found?

@metajack
Copy link
Contributor

@metajack metajack commented Jun 19, 2017

According to http://build.servo.org/builders/windows-msvc-dev/builds/3134 we aren't running wpt tests in automation. I thought we were.

That error is similar to one that I was sure we fixed. This looks like the one I'm thinking of: w3c/wpt-tools#163

@metajack
Copy link
Contributor

@metajack metajack commented Jun 19, 2017

Here's the patch I made back then: w3c/wpt-tools#139

@camlorn
Copy link
Contributor Author

@camlorn camlorn commented Jun 19, 2017

It looks like we're maintaining a local copy. Is this correct? Is the goal here to have a PR both against us and them?

The repository for WPT has moved if it matters and anyone here didn't know.

@jdm
Copy link
Member

@jdm jdm commented Jun 19, 2017

A PR against Servo is fine. It will be easier to figure out if/how it needs to be upstreamed once it's working locally; it's a complicated situation right now.

@camlorn
Copy link
Contributor Author

@camlorn camlorn commented Jun 20, 2017

Is all of WPT supposed to pass? Given what it is, I'm thinking not.

If this is indeed the case, I've got a hacky workaround fix for this part of it.

@metajack
Copy link
Contributor

@metajack metajack commented Jun 20, 2017

If we've never run test-wpt in automation, I expect the normal test expectations aren't sufficient to pass on Windows. When I was working on this last year, I was focused on running specific tests related to the feature I was implementing.

The next step after getting tests to run would be to try and get expectations set correctly, bugs filed for things that are failing and shouldn't (ie, non-platform-specific failures), and finally turning on the test-wpt step in automation so it stays working and keeps improving.

@metajack
Copy link
Contributor

@metajack metajack commented Jun 20, 2017

Those are obviously follow up bugs to this one, just to be clear.

@camlorn camlorn mentioned this issue Jun 20, 2017
4 of 4 tasks complete
bors-servo added a commit that referenced this issue Jun 21, 2017
Windows can run WPT

<!-- Please describe your changes on the following line: -->
This PR contains fixes that address the specific errors in #17392: WTP workarounds for Windows terminals (see #8313), modifications to `mach test --all` that allow `test_tidy` to be called properly, and modifications to the test harness for WPT that allow it to run when OpenSSL isn't found.  How to handle OpenSSL on Windows is probably a separate issue because it doesn't have a straightforward answer: we either need to provide specific instructions to users or bundle it in binary form ourselves.

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `__` with appropriate data: -->
- [x] `./mach build -d` does not report any errors
- [x] `./mach test-tidy` does not report any errors
- [x] These changes fix #17392  (github issue number if applicable).

<!-- Either: -->
- [x] These changes do not require tests because they fix test running.

<!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.-->

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/17419)
<!-- Reviewable:end -->
bors-servo added a commit that referenced this issue Jun 22, 2017
Windows can run WPT

<!-- Please describe your changes on the following line: -->
This PR contains fixes that address the specific errors in #17392: WTP workarounds for Windows terminals (see #8313), modifications to `mach test --all` that allow `test_tidy` to be called properly, and modifications to the test harness for WPT that allow it to run when OpenSSL isn't found.  How to handle OpenSSL on Windows is probably a separate issue because it doesn't have a straightforward answer: we either need to provide specific instructions to users or bundle it in binary form ourselves.

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `__` with appropriate data: -->
- [x] `./mach build -d` does not report any errors
- [x] `./mach test-tidy` does not report any errors
- [x] These changes fix #17392  (github issue number if applicable).

<!-- Either: -->
- [x] These changes do not require tests because they fix test running.

<!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.-->

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/17419)
<!-- Reviewable:end -->
bors-servo added a commit that referenced this issue Jun 22, 2017
Windows can run WPT

<!-- Please describe your changes on the following line: -->
This PR contains fixes that address the specific errors in #17392: WTP workarounds for Windows terminals (see #8313), modifications to `mach test --all` that allow `test_tidy` to be called properly, and modifications to the test harness for WPT that allow it to run when OpenSSL isn't found.  How to handle OpenSSL on Windows is probably a separate issue because it doesn't have a straightforward answer: we either need to provide specific instructions to users or bundle it in binary form ourselves.

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `__` with appropriate data: -->
- [x] `./mach build -d` does not report any errors
- [x] `./mach test-tidy` does not report any errors
- [x] These changes fix #17392  (github issue number if applicable).

<!-- Either: -->
- [x] These changes do not require tests because they fix test running.

<!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.-->

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/17419)
<!-- Reviewable:end -->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

3 participants
You can’t perform that action at this time.