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

Restore log level after running tests that modify it. #202

Merged
merged 2 commits into from
Jul 10, 2021

Conversation

lardieri
Copy link
Contributor

@lardieri lardieri commented Mar 8, 2021

Resolve #201

Certain tests in the suite deliberately provoke warnings.

  • site_github_munger_spec.rb:
    GitHub Metadata: site.name is set in _config.yml, but many plugins and themes expect site.title to be used instead.
  • repository_finder_spec.rb:
    Not Installed: No such file or directory - git

However, other tests set the log level to Error, which suppresses warnings:

  • edit_link_tag_spec.rb
  • integration_spec.rb

Rspec randomizes the order of the tests. If a random seed is chosen that orders the warning tests ahead of the error tests, then the warnings get logged correctly. Example: --seed 41261

However, if the random seed orders either of the error tests ahead of the warning tests, then the warnings get suppressed. Example: --seed 60792

This commit makes the error tests remember the original log level and restore it after they run. Now the warnings will get logged every time.

@lardieri
Copy link
Contributor Author

lardieri commented Mar 8, 2021

All three expected test warnings are present in the console log for the most recent AppVeyor run above:
https://ci.appveyor.com/project/jekyll/github-metadata/builds/38109706/job/k077phiqhin9mn82

If you run the test suite on the master branch with the same seed (--seed 47529), the warnings are suppressed. So you can see the effectiveness of this change.

Certain tests in the suite deliberately provoke warnings.

* site_github_munger_spec.rb: GitHub Metadata: site.name is set in _config.yml, but many plugins and themes expect site.title to be used instead.
* repository_finder_spec.rb: Not Installed: No such file or directory - git

However, other tests set the log level to Error, which suppresses warnings:
* edit_link_tag_spec.rb
* integration_spec.rb

Rspec randomizes the order of the tests. If a random seed is chosen that orders the warning tests ahead of the error tests, then the warnings get logged correctly. Example: --seed 41261

However, if the random seed orders either of the error tests ahead of the warning tests, then the warnings get suppressed. Example: --seed 60792

This commit makes the error tests remember the original log level and restore it after they run. Now the warnings will get logged every time.

Resolve jekyll/github-metadata jekyll#201
@lardieri
Copy link
Contributor Author

@mattr- @parkr How do you feel about this change?

spec/integration_spec.rb Outdated Show resolved Hide resolved
@lardieri lardieri requested a review from parkr July 10, 2021 14:54
@parkr
Copy link
Member

parkr commented Jul 10, 2021

Thank you so much!

@jekyllbot: merge +dev

@jekyllbot jekyllbot merged commit d4b5e71 into jekyll:master Jul 10, 2021
jekyllbot added a commit that referenced this pull request Jul 10, 2021
@jekyll jekyll locked and limited conversation to collaborators Jul 10, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Restore log level after running tests that modify it.
3 participants