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

Make it easier to identify why the grind test might be failing and increase timeout for truffleruby #214

Merged
merged 2 commits into from
Jun 14, 2023

Conversation

rwstauner
Copy link
Contributor

This adds some additional checks and messages to help identify why the test is failing and what might help.

When I first ran the suite with TruffleRuby I was seeing

This worker is exiting early because it reached its timeout of 1 seconds
  test_grind_max_time                                            ERROR (8.89s)
Minitest::UnexpectedError:         NoMethodError: undefined method `scan' for nil:NilClass
            /home/spin/src/github.com/Shopify/ci-queue/ruby/test/integration/grind_redis_test.rb:131:in `test_grind_max_time'

This PR:

  • captures that STDERR line
  • asserts that it is present (to ensure that the subprocess exited for the timeout as expected)
  • protects against nil error with a few more assertions
  • adds a message about what might be happening and what might help
  • measures the time the process took for when its useful to know (based on ruby versions i can get a range including ~2s, ~7s, ~17s)
  • increases the timeout for when using truffleruby to something that will allow the subprocess to get started so that the tests pass

@rwstauner rwstauner self-assigned this May 16, 2023
Copy link
Contributor

@casperisfine casperisfine left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't see a big value in this.

Like o that was your issue with Truffle, but what is the chance someone else run into the exact same issue in the future?

ruby/test/integration/grind_redis_test.rb Outdated Show resolved Hide resolved
ruby/test/integration/grind_redis_test.rb Outdated Show resolved Hide resolved
@rwstauner
Copy link
Contributor Author

I've run into enough tests that fail or error without providing any hints as to what might be wrong
that when i come upon them I like to make them clearer for the next person.
If there was a reason that it didn't pass this time, then it could probably happen again.

It was a while back but I think I originally went down a longer (and less fruitful) road trying to understand what might be wrong (when I thought there might be some other reason that the subprocess wasn't working) before I tried simply increasing the timeout.

@rwstauner rwstauner merged commit 9946dc1 into master Jun 14, 2023
5 of 6 checks passed
@rwstauner rwstauner deleted the rwstauner/grind branch June 14, 2023 17:45
@shopify-shipit shopify-shipit bot temporarily deployed to rubygems June 14, 2023 17:47 Inactive
@rwstauner
Copy link
Contributor Author

just got this on a branch in CI which was very helpful to see
image

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

3 participants