Skip to content

Test examples in user guide with travis #387

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

Merged
merged 1 commit into from
Mar 10, 2019

Conversation

Alexander-N
Copy link
Member

@Alexander-N Alexander-N commented Mar 4, 2019

@Alexander-N Alexander-N changed the title Test examples in user guide with travis WIP: Test examples in user guide with travis Mar 4, 2019
@@ -51,6 +51,8 @@ features = ["extension-module"]
**`src/lib.rs`**

```rust
extern crate pyo3;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add a note in the readme that extern crate pyo3; is not required when using the 2018 edition

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

@@ -182,7 +182,7 @@ fn impl_wrap_init(cls: &syn::Type, name: &syn::Ident, spec: &FnSpec<'_>) -> Toke
unsafe extern "C" fn __wrap(
_slf: *mut ::pyo3::ffi::PyObject,
_args: *mut ::pyo3::ffi::PyObject,
_kwargs: *mut ::pyo3::ffi::PyObject) -> libc::c_int
_kwargs: *mut ::pyo3::ffi::PyObject) -> ::std::os::raw::c_int
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this required? I would really like to avoid having to stick with the 2015 edition for the derive only for the doctest. (We should be able pass --extern=2018 to rustdoc to avoid this if it doesn't work otherwise)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Even when passing the edition directly to rustdoc, I get

---- guide/src/class.md - Python_Class::Object_properties (line 232) stdout ----
error[E0433]: failed to resolve: use of undeclared type or module `libc`
  --> guide/src/class.md:240:1
   |
10 | #[pymethods]
   | ^^^^^^^^^^^^ use of undeclared type or module `libc`

thread 'guide/src/class.md - Python_Class::Object_properties (line 232)' panicked at 'couldn't compile the test', src/librustdoc/test.rs:354:13
note: Run with `RUST_BACKTRACE=1` environment variable to display a backtrace.

---- guide/src/class.md - Python_Class::Object_properties (line 261) stdout ----
error[E0433]: failed to resolve: use of undeclared type or module `libc`
  --> guide/src/class.md:269:1
   |
10 | #[pymethods]
   | ^^^^^^^^^^^^ use of undeclared type or module `libc`

thread 'guide/src/class.md - Python_Class::Object_properties (line 261)' panicked at 'couldn't compile the test', src/librustdoc/test.rs:354:13

---- guide/src/class.md - Python_Class::Object_properties (line 289) stdout ----
error[E0433]: failed to resolve: use of undeclared type or module `libc`
 --> guide/src/class.md:292:1
  |
5 | #[pyclass]
  | ^^^^^^^^^^ use of undeclared type or module `libc`

thread 'guide/src/class.md - Python_Class::Object_properties (line 289)' panicked at 'couldn't compile the test', src/librustdoc/test.rs:354:13

@konstin
Copy link
Member

konstin commented Mar 4, 2019

Thank you! I've left two comments, otherwise it's a that's a big improvement!

@Alexander-N
Copy link
Member Author

On travis I'm getting weird linking errors like /usr/bin/ld: cannot find -lpython3.6m. Any help would be great.

Alexander-N added a commit to Alexander-N/pyo3 that referenced this pull request Mar 5, 2019
@konstin
Copy link
Member

konstin commented Mar 7, 2019

I don't know the exact problem or the solution, but I can give some context: To run the tests, we need to build binaries, which needs to link libpython in the version specified by the build script. Normally the build script also tells the linker the path of libpyhon through rustc-link-search:

pyo3/build.rs

Lines 444 to 448 in d63fac8

if libpath != "None" {
println!("cargo:rustc-link-search=native={}", libpath);
} else if cfg!(target_os = "windows") {
println!("cargo:rustc-link-search=native={}\\libs", exec_prefix);
}

This seems to fail here, presumably because the we're calling rustdoc here directly (through docmatic) and not cargo.

It's also possible to add directories to the linker path using environment variables, but that should have already been done by the script that travis runs:

pyo3/ci/travis/setup.sh

Lines 16 to 20 in d63fac8

PYTHON_LIB=$(python -c "import sysconfig; print(sysconfig.get_config_var('LIBDIR'))")
export LD_LIBRARY_PATH="$LD_LIBRARY_PATH:$PYTHON_LIB:$HOME/rust/lib"
echo ${LD_LIBRARY_PATH}

The logs even tell that this path has been set: https://travis-ci.org/PyO3/pyo3/jobs/502189479#L322

I am not really sure what to do here because debugging travis is a tedious task. The quick and dirty solution would be to make the minimum nightly use python 3.5 (which apparently works) and then only run those test in the minimum nightly builds, just like rustfmt and clippy.

Alexander-N added a commit to Alexander-N/pyo3 that referenced this pull request Mar 9, 2019
Test could only be activated for Python 3.5 and some tests had to be
ignored, see PyO3#381 and PyO3#387.
@Alexander-N
Copy link
Member Author

Ok, then I would suggest we activate the tests only for Python 3.5 for now. This way we have at least some mechanism in place which prevents the examples to drift from the current API. We could still leave #385 open and look for a better solution.

@Alexander-N Alexander-N changed the title WIP: Test examples in user guide with travis Test examples in user guide with travis Mar 9, 2019
@konstin
Copy link
Member

konstin commented Mar 10, 2019

I think that eventually, we want to either use a fix version of skeptic/docmatic or (which is more likely) write our own small doc tester. But for now this is a really good solution for moving forward with the docs.

@konstin konstin merged commit ba5bc0f into PyO3:master Mar 10, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants