Skip to content
This repository has been archived by the owner on Jan 26, 2022. It is now read-only.

Add new test for pagination variables #415

Merged
merged 4 commits into from
Apr 24, 2020

Conversation

KrzysztofSikoraCodete
Copy link
Contributor

Peek 2020-04-02 11-45

I've just added test for paging request. The test could pass If the filtered variables result were less than 2. Now we have more than 1 even though we use pagination.

I checked session configuration and supportsVariablePaging, has true.

Any suggestions?

@afshin
Copy link
Member

afshin commented Apr 2, 2020

Great, I think this is a sufficient test case to work against on the kernel side. Could you please open an issue in https://github.com/jupyter-xeus/xeus-python/ and tag this issue there?

Cheers, @KrzysztofSikoraCodete!

@jtpio
Copy link
Member

jtpio commented Apr 2, 2020

Thanks @KrzysztofSikoraCodete for looking into this 👍

@JohanMabille
Copy link
Member

jupyter-xeus/xeus-python#246 should fix it.

@jtpio
Copy link
Member

jtpio commented Apr 6, 2020

Thanks @JohanMabille!

@afshin
Copy link
Member

afshin commented Apr 7, 2020

Awesome, thank you @JohanMabille!

@jtpio
Copy link
Member

jtpio commented Apr 23, 2020

Just re-triggered the tests and it looks like the conda tests are now passing with the latest xeus-python.

However the wheel is giving some errors: https://github.com/jupyterlab/debugger/pull/415/checks?check_run_id=612276325

@jtpio
Copy link
Member

jtpio commented Apr 24, 2020

See jupyter-xeus/xeus-python-wheel#54 for more context.

The JupyterLab jest tests patch the jupyter paths here: https://github.com/jupyterlab/jupyterlab/blob/36a8da4d6bffea0fd9992a4d88627cffb46fe6a0/jupyterlab/tests/test_app.py#L88-L93

With the data directory becoming something like /tmp/tmpgrqb54bk/jupyter_data.

Which is why we install the xpython spec explicitly in the tests here:

jest_app.install_kernel(
kernel_name='xpython',
kernel_spec={
'argv': [
'xpython',
'-f', '{connection_file}'
],
'display_name': 'xpython',
'language': 'python'
}
)

@jtpio
Copy link
Member

jtpio commented Apr 24, 2020

As discussed, let's disable the wheel tests for now and re-enable them in a separate PR.

@jtpio
Copy link
Member

jtpio commented Apr 24, 2020

Thanks!

@jtpio jtpio merged commit bf5a287 into jupyterlab:master Apr 24, 2020
@jtpio jtpio mentioned this pull request Apr 24, 2020
@KrzysztofSikoraCodete
Copy link
Contributor Author

Thanks for merge! Nice to heard that.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants