Update to use Minitest 5.15.0 #217

MSP-Greg · 2024-01-03T16:12:59Z

Tried with new version and SU 2020 (Ruby 2.5.5), seemed to work.

EDIT: Added 2nd commit to fix SU 2017 seemed to work, had to rename TC_Geom_Point2d.rb, TC_Geom_Transformation2d.rb, and TC_Geom_Vector2d.rb

Please test, as I'm not that familiar with TestUp...

MSP-Greg · 2024-01-03T16:27:04Z

2nd commit allows TestUp to load with SU 2017.

thomthom · 2024-01-03T16:53:46Z

Thanks for looking into this. I'll have to check out the branch and run some tests on various SU versions.

thomthom

Would also have to update README.me, but I can have a look at that afterwards.

thomthom · 2024-01-03T16:50:20Z

tests/SketchUp Ruby API/TC_Geom_Point2d.rb

@@ -2,6 +2,7 @@
 # License:: The MIT License (MIT)
 # Original Author:: Adam Karkkainen

+if Geom.const_defined? :Point2d


What are these conditionals for?

Oh, for running on older SketchUp versions?

Oh, for running on older SketchUp versions?

Yes. I wanted to see if it worked with Ruby 2.2, so I tried SU 2017.

The '2d' objects were new with SU 2018, JFYI, doc's refer to Layout (not SU) in their minimum version item.

Yea, the 2d classes was introduced because of LayOut. SketchUp never really used them. I think we might have introduced some usage of them later on though.

thomthom · 2024-01-03T16:52:28Z

src/testup/core.rb

@@ -9,7 +9,9 @@

 # TODO: Defer this to a point so it doesn't happen during SketchUp startup.
 require 'testup/gem_helper'
-TestUp::GemHelper.require('minitest', 'minitest', version: '5.4.3')
+
+# Minitest 5.15.0 is compatible with Ruby '>= 2.2, < 4.0', released 15-Dec-2021


With that the .rubocop.yml should update TargetSketchUpVersion and TargetRubyVersion.

I wasn't sure what you wanted to do, I don't think I have a complete set of old Ruby 2.2. SU versions.

I can have a look at the rubocop config.

thomthom · 2024-01-03T16:55:24Z

I just realized you had a PR from 2018 related to this (#104). Never got around to dig into that one. (Sorry!)

thomthom · 2024-01-03T16:57:11Z

Wondering if it's worth pulling in the fix I have for 5.4.3:
https://github.com/SketchUp/testup-2/pull/214/files

Making use of the expected gem version when trying to activate it makes sense, right?

MSP-Greg · 2024-01-03T17:53:37Z

Making use of the expected gem version when trying to activate it makes sense, right?

See the third commit. This used the pre-existing Minitest from a new SU version, and installed 5.15.0 using SU 2023, even though it had Minitest 5.4.3 installed from an older TestUp.

So, if the SU version has a version of Minitest installed that is 5.15.0 or later, activate it, otherwise, install it. Without the change in the third commit, it would activate any version of Minitest...

MSP-Greg · 2024-01-04T00:27:46Z

@thomthom

I thought I'd check the node version issue. This does build with Node.js 16. See:

https://github.com/MSP-Greg/testup-2/actions/runs/7403666596

I installed the artifact on the new SU version, and it seems to work. Node.js 18 failed. Node.js 10 is rather old.

thomthom · 2024-01-04T10:57:27Z

@thomthom

I thought I'd check the node version issue. This does build with Node.js 16. See:

https://github.com/MSP-Greg/testup-2/actions/runs/7403666596

I installed the artifact on the new SU version, and it seems to work. Node.js 18 failed. Node.js 10 is rather old.

It's old, yes. But I don't have the time right now to deal with npm updates. It's always a rabbit hole and a time sink, which I don't have time for right now. Node 10.4 was the last version used that worked successfully, using that until we can find time to upgrade.

Your PR appear to be stuck from a point where I was working on the GitHub actions. If you rebase to the tip of main you'll get the latest actions working. https://github.com/SketchUp/testup-2/actions

thomthom · 2024-01-04T10:58:11Z

src/testup/gem_helper.rb

@@ -14,7 +14,7 @@ module GemHelper
    def self.require(gem_name, filename, version: Gem::Requirement.default)
      Kernel.require 'rubygems'
      begin
-        gem gem_name
+        gem gem_name, ">=#{version}"


Ah, nice. didn't realize you could use a flexible version here.

MSP-Greg · 2024-01-04T14:28:03Z

It's old, yes. But I don't have the time right now to deal with npm updates. It's always a rabbit hole and a time sink, which I don't have time for right now. Node 10.4 was the last version used that worked successfully, using that until we can find time to upgrade.

npm updates are different than node.js updates. Feel free to try the extension. It builds, and it seems to work, except for some issues running tests in the SU console. Didn't have time to check on SU 2022, which doesn't have the write bug.

Your PR appear to be stuck from a point where I was working on the GitHub actions. If you rebase

Sorry, I was working in the branch with the node update (which was rebased), pushed the rebase here

thomthom · 2024-01-04T14:32:26Z

I tested SU2017 and that worked fine.

except for some issues running tests in the SU console

What issues is that?

Didn't have time to check on SU 2022, which doesn't have the write bug.

That's related to "running tests in the SU console"?

MSP-Greg · 2024-01-04T14:36:13Z

That's related to "running tests in the SU console"?

For instance, when running Minitest without -v, output is a line of dot characters for every passing test. When I ran console tests with a new version, it was a dot character per line.

Not sure how to get SU console output matching the log file (setting -v for Minitest), but I couldn't get the console to match.

thomthom · 2024-01-04T15:13:14Z

For instance, when running Minitest without -v, output is a line of dot characters for every passing test. When I ran console tests with a new version, it was a dot character per line.

Right, but that's a bug in SketchUp SU2023.1, right? Not a result of the new Minitest version?

thomthom · 2024-01-04T15:15:19Z

I'm seeing the console result issue in SU2023.1 with Minitest 5.4.3, but not in earlier versions. So that's an unrelated issue.

I think this is ready to merge. But I'll run some extra tests. Will probably wait until monday.

MSP-Greg · 2024-01-04T16:06:49Z

I think this is ready to merge. But I'll run some extra tests. Will probably wait until monday.

Works for me. I should be around this weekend.

I got it running in SU 2022. Using the menu, with 'Run tests in Ruby Console' checked and 'Verbose Console Tests' unchecked, I get the 'line of dots', as expected.

When I check 'Verbose Console Tests', there's no change. I'll try to have a look at it, if you find a fix. please let me know.

thomthom · 2024-01-04T16:31:50Z

I'll be looking into the new-line Ruby Console issue on Windows next week.

MSP-Greg · 2024-01-05T19:21:47Z

For this PR, I think I worked from a release, and copied the changes back into the repo.

Now, I installed nodejs 16.20.2, built it, and I've got the repo symlinked into SU 2017 & 2022. With some changes, both run correctly in the console with 'Verbose Console Tests' set on or off. They also correctly run the tests in the GUI.

I can add the commits to this, or save for another PR after this is merged.

MSP-Greg · 2024-01-06T23:27:46Z

Added one commit which speeds up the local install of the included gem file.

EDIT: tested with SU 2017 & SU 2022

thomthom · 2024-01-08T10:28:26Z

src/testup/gem_helper.rb

@@ -58,7 +58,8 @@ def self.install(gem_name, version = Gem::Requirement.default)
      if local_path
        puts '> Installing from local copy...'
        puts "> #{local_path}"
-        Gem.install(local_path)
+        Gem.install(local_path, nil, {domain: :local})


This domain: local options, how far back is that supported?

Ruby 2.2, SU 2017. It doesn't exist in Ruby 2.0, not sure about 2.1

MSP-Greg · 2024-01-08T14:14:06Z

Should a SketchUp version guard be added to src/testup.rb?
Update the README.md have a section added about what TestUp versions are compatible with various SU versions?
I've got a commit in another branch .rubocop.yml - SU 2017 and Ruby 2.2, should I add it to this PR?
What version of TestUp will contain these updates? 2.5.0?

thomthom · 2024-01-08T14:25:12Z

I can take care of the version guard, README etc.

thomthom reviewed Jan 3, 2024

View reviewed changes

thomthom reviewed Jan 4, 2024

View reviewed changes

MSP-Greg added 3 commits January 4, 2024 08:16

Update to use Minitest 5.15.0

a39f5e1

Guard Geom 2d objects (SU 2018?)

9c5b74a

gem_helper.rb - fixup to check Minitest version

056f265

MSP-Greg force-pushed the 00-minitest-update branch from 1a2c7d5 to 056f265 Compare January 4, 2024 14:17

gem_helper.rb - use '--local' equivalent for install

753f02a

thomthom reviewed Jan 8, 2024

View reviewed changes

MSP-Greg mentioned this pull request Jan 8, 2024

Fix class name in reports #104

Closed

thomthom merged commit 936dfb7 into SketchUp:main Jan 9, 2024
3 checks passed

thomthom mentioned this pull request Jan 9, 2024

Make GemHelper use the provided version. #214

Closed

MSP-Greg deleted the 00-minitest-update branch January 9, 2024 13:37

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update to use Minitest 5.15.0 #217

Update to use Minitest 5.15.0 #217

MSP-Greg commented Jan 3, 2024 •

edited

Loading

MSP-Greg commented Jan 3, 2024

thomthom commented Jan 3, 2024

thomthom left a comment

thomthom Jan 3, 2024

thomthom Jan 3, 2024

MSP-Greg Jan 3, 2024

thomthom Jan 4, 2024

thomthom Jan 3, 2024

MSP-Greg Jan 3, 2024

thomthom Jan 4, 2024

thomthom commented Jan 3, 2024

thomthom commented Jan 3, 2024

MSP-Greg commented Jan 3, 2024 •

edited

Loading

MSP-Greg commented Jan 4, 2024

thomthom commented Jan 4, 2024

thomthom Jan 4, 2024

MSP-Greg commented Jan 4, 2024

thomthom commented Jan 4, 2024 •

edited

Loading

MSP-Greg commented Jan 4, 2024

thomthom commented Jan 4, 2024

thomthom commented Jan 4, 2024 •

edited

Loading

MSP-Greg commented Jan 4, 2024

thomthom commented Jan 4, 2024

MSP-Greg commented Jan 5, 2024

MSP-Greg commented Jan 6, 2024 •

edited

Loading

thomthom Jan 8, 2024

MSP-Greg Jan 8, 2024

MSP-Greg commented Jan 8, 2024

thomthom commented Jan 8, 2024

Update to use Minitest 5.15.0 #217

Update to use Minitest 5.15.0 #217

Conversation

MSP-Greg commented Jan 3, 2024 • edited Loading

MSP-Greg commented Jan 3, 2024

thomthom commented Jan 3, 2024

thomthom left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

thomthom commented Jan 3, 2024

thomthom commented Jan 3, 2024

MSP-Greg commented Jan 3, 2024 • edited Loading

MSP-Greg commented Jan 4, 2024

thomthom commented Jan 4, 2024

Choose a reason for hiding this comment

MSP-Greg commented Jan 4, 2024

thomthom commented Jan 4, 2024 • edited Loading

MSP-Greg commented Jan 4, 2024

thomthom commented Jan 4, 2024

thomthom commented Jan 4, 2024 • edited Loading

MSP-Greg commented Jan 4, 2024

thomthom commented Jan 4, 2024

MSP-Greg commented Jan 5, 2024

MSP-Greg commented Jan 6, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

MSP-Greg commented Jan 8, 2024

thomthom commented Jan 8, 2024

MSP-Greg commented Jan 3, 2024 •

edited

Loading

MSP-Greg commented Jan 3, 2024 •

edited

Loading

thomthom commented Jan 4, 2024 •

edited

Loading

thomthom commented Jan 4, 2024 •

edited

Loading

MSP-Greg commented Jan 6, 2024 •

edited

Loading