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

Hide warning output when running specs #184

Closed
faultyserver opened this issue Apr 7, 2018 · 3 comments
Closed

Hide warning output when running specs #184

faultyserver opened this issue Apr 7, 2018 · 3 comments
Labels
good first issue An issue that provides a good intro to working with the Myst codebase. Be helpful! spec An issue with relating to the specs around the Myst codebase (native or in-language).
Milestone

Comments

@faultyserver
Copy link
Member

screen shot 2018-04-07 at 7 31 45 pm

Parser warnings that come up from various specs have been appearing in the output for a while now. I think they used to be hidden by a MYST_ENV variable being set to test, but that isn't happening anymore.

The proper solution for this is to use interpret_with_mocked_output for these specs so that the output is captured and can be tested properly (right now the specs just check if a warning was raised, not what that warning was).

Most of the offending specs are probably using the it_warns helper method defined here:

def it_warns(source, expected, file=__FILE__, line=__LINE__, end_line=__END_LINE__)
it "raises warning from `#{source}`", file, line, end_line do
interpreter=Interpreter.new
program = parse_program(source)
interpreter.run(program)
interpreter.warnings.should eq expected
end
end

The fix would be to call interpret_with_mocked_output(source) from this method instead of creating a new Interpreter and running it directly, then checking the warnings from the interpreter object returned by that method.

@faultyserver faultyserver added good first issue An issue that provides a good intro to working with the Myst codebase. Be helpful! spec An issue with relating to the specs around the Myst codebase (native or in-language). labels Apr 7, 2018
@faultyserver
Copy link
Member Author

It actually looks like these messages are coming from vm_spec.cr where it's testing the standard IO usage. I'm not sure what the best approach to solving this is.

@Jens0512
Copy link
Member

Jens0512 commented Apr 9, 2018

What makes you think it comes from vm_spec.cr? Code containing underscore vars is not run in the VM specs. And we really should rename VM 😅 ... I clearly remember dealing with just this issue before, I might have forgotten to push it here though, or I i did, and it somehow got broken.

@faultyserver
Copy link
Member Author

Well, I commented out everything in vm_spec because a test was failing (I had made some radical changes at the time) and the warning output didn't appear.

I think this is the offending line:

ENV["MYST_ENV"] = product? ? "prod" : "test"

It assigns the ENV variable, which will persist for the duration of the test suite. So even if the tests that actually output warnings to the terminal are in another file, this should be addressed to avoid leaking.

I think the best solution would be to remove that line entirely. The VM shouldn't be responsible for managing environment variables.

@faultyserver faultyserver added this to the Next milestone Apr 17, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue An issue that provides a good intro to working with the Myst codebase. Be helpful! spec An issue with relating to the specs around the Myst codebase (native or in-language).
Projects
None yet
Development

No branches or pull requests

2 participants