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

Don't return resources #610

Merged
merged 2 commits into from
May 20, 2020

Conversation

davidbrochart
Copy link
Member

This led to the following error when a cell was erroring out:

jinja2.exceptions.UndefinedError: 'tuple object' has no attribute 'get'

@maartenbreddels
Copy link
Member

I'd like to have a test for this, do you feel like doing that?

@davidbrochart
Copy link
Member Author

It's tricky because a test with e.g. a syntax error in a cell doesn't fail, and doesn't show the error in stdout or stderr as if Voila were run on the CLI.

@maartenbreddels
Copy link
Member

It should give this error msg in red right? Run with --debug .... I think that feature isn't really tested at all.

@davidbrochart
Copy link
Member Author

davidbrochart commented May 20, 2020

We want to test the presence of 'tuple object' has no attribute 'get', but it's not there with pytest.

@maartenbreddels
Copy link
Member

I think we don't have to test that detailed. By default, voila should render a clean error message, now it does not do that. So I think if we just add a new test notebook, with say an import error, or syntax error, we can test if the rendered HTML contains that nice error message. What do you think about that?

@davidbrochart
Copy link
Member Author

Sounds good, I added a test for that and checked that it was failing before this PR.

@maartenbreddels maartenbreddels merged commit 80b120c into voila-dashboards:master May 20, 2020
@maartenbreddels maartenbreddels deleted the cell_error branch May 20, 2020 16:45
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.

None yet

2 participants