SketchUp / testup-2

TestUp 2 for SketchUp - A GUI wrapper for running Minitest in SketchUp
MIT License
26 stars 37 forks source link

Update to use Minitest 5.15.0 #217

Closed MSP-Greg closed 10 months ago

MSP-Greg commented 11 months ago

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 commented 11 months ago

2nd commit allows TestUp to load with SU 2017.

thomthom commented 11 months ago

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

thomthom commented 11 months ago

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

thomthom commented 11 months ago

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 commented 11 months ago

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 commented 11 months ago

@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 commented 11 months ago

@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

MSP-Greg commented 11 months ago

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 commented 11 months ago

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 commented 11 months ago

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 commented 11 months ago

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 commented 11 months ago

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 commented 11 months ago

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 commented 11 months ago

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

MSP-Greg commented 11 months ago

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 commented 10 months ago

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

EDIT: tested with SU 2017 & SU 2022

MSP-Greg commented 10 months ago
  1. Should a SketchUp version guard be added to src/testup.rb?

  2. Update the README.md have a section added about what TestUp versions are compatible with various SU versions?

  3. I've got a commit in another branch .rubocop.yml - SU 2017 and Ruby 2.2, should I add it to this PR?

  4. What version of TestUp will contain these updates? 2.5.0?

thomthom commented 10 months ago

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