Skip to content
This repository has been archived by the owner on May 7, 2021. It is now read-only.

Configurable Guru Timeout #951

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

robbawebba
Copy link

@robbawebba robbawebba commented Oct 7, 2019

This commit adds a user-configurable timeout value to all guru commands.

closes #951

Two concerns I would like to address about the proposed changes:

  • I am encountering an issue with guru when running tests locally (both on my branch and on the HEAD of master): guru: no initial packages were loaded. I'm unsure if this is due to my local dev environment, so I wanted to see if the tests pass in the CI environment.
  • It is proving to be very difficult to test the highlight provider guru timeout. this guru operation seems to work pretty quickly in the test environment. When I set a timeout of 1ms in the package settings during normal usage, the command does not seem to finish within 1ms and no identifiers in the editor are highlighted. when I set a timeout of 1ms in a spec, the highlight provider's highlight function seems to finish before the timeout, and valid highlight ranges are returned by the function. I'm not sure if it's going to be possible to test the timeout of the highlight provider.

@robbawebba robbawebba force-pushed the add-configurable-guru-timeout branch from d97a3d5 to 50d2875 Compare October 7, 2019 05:21
@robbawebba robbawebba force-pushed the add-configurable-guru-timeout branch from 50d2875 to 5ecd342 Compare October 7, 2019 05:54
@zmb3
Copy link
Collaborator

zmb3 commented Oct 7, 2019

Looks like a couple tests are broken, but otherwise LGTM.

@robbawebba
Copy link
Author

Hi @zmb3, thanks for taking a look!
I'm running into issued with running the specs locally, which makes the issues difficult to debug. I'm running several issues with guru locally that do not seem to be a problem in the CI environment.
For example, most guru commands that are run through the specs fail with this error:

guru: no initial packages were loaded

Have you seen this before while running tests locally?

@zmb3
Copy link
Collaborator

zmb3 commented Oct 15, 2019

I haven't seen that locally, though I admit it's been a while since I've tried. That sounds to me like a gopath or working directory problem.

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.

2 participants