Update to CodeObjects 'BUILTIN' constants #1005

MSP-Greg · 2016-07-29T23:14:07Z

Description

Per #1003. Additionally, I decided to update all three, BUILTIN_EXCEPTIONS, BUILTIN_CLASSES, and BUILTIN_MODULES.

Ordered items
Switched to %w
Ruby 2.3.2
Left MatchingData in BUILTIN_CLASSES
BUILTIN_CLASSES still doesn't contain ARGF & ENV , both fail RSpec
Kept approx 80+ char margin
Blank space between ok?

Completed Tasks

I have read the Contributing Guide.
The pull request is complete (implemented / written).
Git commits have been cleaned up (squash WIP / revert commits).
I wrote tests and they pass (if code is attached to PR).

coveralls · 2016-07-29T23:15:41Z

Coverage remained the same at 93.519% when pulling 2a4dedc on MSP-Greg:master into 9be34c6 on lsegal:master.

lsegal · 2016-07-29T23:33:11Z

Tests don't seem to be passing on earlier Ruby versions (specs probably need updating), and I see some comment formatting errors. I think we should just start with the added constants to deal with #1003 directly and then open a refactoring PR afterwards if you want to change the coding style so as to not conflate these two changes.

MSP-Greg · 2016-07-29T23:59:52Z

Sorry about that comment, my damn editor does that (add a 'spacer' line, and it indents the next). Missed it.

Re the tests, it seems the only error is related to ClosedQueueError, which is new in 2.3 and not defined in 2.2 or earlier (I've got 2.0, 2.2, 2.3, and 2.4 doc'd).

The two tests expect all BUILTIN members to be defined in the running version of Ruby.

expect(eval(name)).to be_instance_of(Class)
expect(eval(name)).to be <= Exception

Should I leave that in?
%w or string array?

Sorry, I didn't look at the tests, I just saw they passed with Ruby 2.3.2.

Also, I've wondered about having multiple versions of Ruby installed, but haven't yet done so. You use Windows? If so, reasonably easy to set up that way?

lsegal · 2016-07-30T00:09:23Z

We need to fix the tests to be lenient regarding missing constants on older versions or the tests will not pass.

Use a string array for now, if you want to refactor, I can look at that separately.

As for running tests, you should be able to see Travis output on your own fork if you set it up. If you want to run locally, I'd recommend installing Docker and just running:

> docker run -v %CD%:/yard -w /yard ruby:2.2 /bin/sh -c "bundle && bundle exec rake"

You can build a docker image with the installed gem dependencies if you test a lot (to avoid bundle installing).

coveralls · 2016-07-30T00:27:36Z

Coverage decreased (-0.006%) to 93.513% when pulling c89a00b on MSP-Greg:master into 9be34c6 on lsegal:master.

MSP-Greg · 2016-07-30T01:44:31Z

FYI, I checked for what's added from the previous code, re RSpec, and came up with the following (ClosedQueueError just happens to be on both lists and is the 1st addition alphabetically) --

new Exception Classes

ClosedQueueError
EncodingError
FiberError
KeyError
StopIteration
UncaughtThrowError

new Classes

Complex
ConditionVariable
Encoding
Enumerator
Fiber
Queue
Random
RubyVM
SizedQueue
TracePoint

MSP-Greg · 2016-07-30T22:10:05Z

@lsegal I'm having a bad git day (stack patch). IMHO, git docs (git-scm, etc) are very good when they address working with one's own repro and a remote origin, but they get sketchy when dealing with forks, etc, especially how to have multiple PR's open between a local fork, a remote origin, and the original repo. I should have never used the master branch for these commits/PR...

Anyway,

We need to fix the tests to be lenient regarding missing constants on older versions or the tests will not pass.

I'm looking over some of the Ruby 2.4 docs, I suspect we need the tests to work both ways, as there may be Classes/Exceptions/Modules that might be deprecated or removed in future versions. Also, the tests currently fail on first failure, it would be nice if they loaded an array with all failures and then tested whether the array was empty, hence, showing all failures, not just the first.

lsegal · 2016-07-30T22:14:56Z

I'm looking over some of the Ruby 2.4 docs, I suspect we need the tests to work both ways, as there may be Classes/Exceptions/Modules that might be deprecated or removed in future versions.

I strongly doubt any class in Ruby will ever go away, regardless of how "deprecated" it is (technically none of those are even deprecated yet). It's fairly safe to keep any existing constant in the tests. Given that it's only a testing functionality (YARD doesn't eval these during runtime), any failures would be caught as we move Travis to new Ruby versions. A RUBY_VERSION check for new constants can be used to exclude the new constants, and same for old ones:

describe 'Ruby 2.4+ constants' do
  it '...' { ... }
end if RUBY_VERSION >= '2.4'

Also, the tests currently fail on first failure, it would be nice if they loaded an array with all failures and then tested whether the array was empty, hence, showing all failures, not just the first.

I'd accept this change. Moving the class loop outside of the it() declaration (one it() block per class) would fix this.

MSP-Greg · 2016-07-31T00:48:12Z

I'm currently working on this. So we'll list constants for 2.3, and test for earlier and later.

Reason for later - Mutex exists in 2.2, changed to Thread::Mutex in 2.3. Yes, I'm (not) having fun. Are you sure you only want to have 2.2.x and 2.3.x in travis.yml? 2.1? 2.0?

coveralls · 2016-07-31T04:42:17Z

Coverage increased (+0.02%) to 93.536% when pulling 0bea41c on MSP-Greg:master into 9be34c6 on lsegal:master.

lsegal · 2016-08-07T22:36:13Z

Running tests on Travis against 2.0 and 2.1 is difficult now with dependency changes (like Rack, etc). Given the EOL for both versions, I'm okay with stopping at a "best effort" to ensure compatibility, and if regressions occur, we can accept patches to fix them. The burden on maintaining CI against those old versions is too high.

MSP-Greg · 2016-08-07T22:54:53Z

@lsegal Eight days ago I was looking at this, but I moved to other issues. Can we revisit this?

I've never closed a PR, if I do so, does that flush everything out? Again, apology for PR'ing on the main branch.

I'd like to PR the stack freeze_tree code, that test string flip may still be around, I've got a patch to the frozen strings 'directive', etc.

lsegal · 2016-08-08T16:15:24Z

spec/code_objects/constants_spec.rb

      YARD::CodeObjects::BUILTIN_EXCEPTIONS.each do |name|
-        expect(eval(name)).to be <= Exception
+         next if name == 'ClosedQueueError' && RUBY_VERSION < '2.3'


Instead of manually mapping out constants (this will be a headache to maintain), why not just ignore any constant that does not exist? A NameError rescue should suffice.

lsegal · 2016-08-08T16:15:55Z

@MSP-Greg I added some comments, but this also needs the commit history cleaned up. You can rebase and force push your branch after you clean it up so that you don't have to close the PR and open a new one. Ideally this should only require 1 single commit.

* Tested and passed locally against 2.2.4 & 2.3.2. * Removed `Continuation` from `BUILTIN_CLASSES`, it's very similar to `ARGF` & `ENV`, no constructor, fails Module.constants.include?(:Continuation), and all doc examples show `require "continuation"` * Removed `MatchingData`, it's a left-over from before 2.0 * Added a bad class for testing, failed tests do show the full array...

coveralls · 2016-08-08T23:16:03Z

Coverage decreased (-0.004%) to 93.463% when pulling 25451cf on MSP-Greg:master into 51326ee on lsegal:master.

MSP-Greg · 2016-08-08T23:21:33Z

Thank you for the Git tips. I generally don't forget once I've done something.

lsegal · 2016-08-08T23:32:36Z

Looks good, thanks!

References: #1002, #1004, #1005, #1006, #1007, #1008, #1009, #1011, #1012, #1016, #1024, #1028, #1033, #1042, #1045, #1047, #1049, #1050, #1051, #1052, #799, #948, #995, #997, docmeta/rubydoc.info#44

lsegal reviewed Aug 8, 2016
View reviewed changes

MSP-Greg force-pushed the master branch from 0bea41c to 25451cf Compare August 8, 2016 23:14

lsegal merged commit 57ebca0 into lsegal:master Aug 8, 2016

lsegal mentioned this pull request Aug 10, 2016

YARD::CodeObjects::BUILTIN_EXCEPTIONS - missing exception classes #1003

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update to CodeObjects 'BUILTIN' constants #1005

Update to CodeObjects 'BUILTIN' constants #1005

MSP-Greg commented Jul 29, 2016 •

edited by lsegal

Loading

coveralls commented Jul 29, 2016 •

edited

Loading

lsegal commented Jul 29, 2016

MSP-Greg commented Jul 29, 2016

lsegal commented Jul 30, 2016 •

edited

Loading

coveralls commented Jul 30, 2016 •

edited

Loading

MSP-Greg commented Jul 30, 2016

MSP-Greg commented Jul 30, 2016

lsegal commented Jul 30, 2016 •

edited

Loading

MSP-Greg commented Jul 31, 2016

coveralls commented Jul 31, 2016 •

edited

Loading

lsegal commented Aug 7, 2016

MSP-Greg commented Aug 7, 2016

lsegal Aug 8, 2016

lsegal commented Aug 8, 2016

coveralls commented Aug 8, 2016 •

edited

Loading

MSP-Greg commented Aug 8, 2016

lsegal commented Aug 8, 2016

Update to CodeObjects 'BUILTIN' constants #1005

Update to CodeObjects 'BUILTIN' constants #1005

Conversation

MSP-Greg commented Jul 29, 2016 • edited by lsegal Loading

Description

Completed Tasks

coveralls commented Jul 29, 2016 • edited Loading

lsegal commented Jul 29, 2016

MSP-Greg commented Jul 29, 2016

lsegal commented Jul 30, 2016 • edited Loading

coveralls commented Jul 30, 2016 • edited Loading

MSP-Greg commented Jul 30, 2016

MSP-Greg commented Jul 30, 2016

lsegal commented Jul 30, 2016 • edited Loading

MSP-Greg commented Jul 31, 2016

coveralls commented Jul 31, 2016 • edited Loading

lsegal commented Aug 7, 2016

MSP-Greg commented Aug 7, 2016

lsegal Aug 8, 2016

Choose a reason for hiding this comment

lsegal commented Aug 8, 2016

coveralls commented Aug 8, 2016 • edited Loading

MSP-Greg commented Aug 8, 2016

lsegal commented Aug 8, 2016

MSP-Greg commented Jul 29, 2016 •

edited by lsegal

Loading

coveralls commented Jul 29, 2016 •

edited

Loading

lsegal commented Jul 30, 2016 •

edited

Loading

coveralls commented Jul 30, 2016 •

edited

Loading

lsegal commented Jul 30, 2016 •

edited

Loading

coveralls commented Jul 31, 2016 •

edited

Loading

coveralls commented Aug 8, 2016 •

edited

Loading