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 cucumber CCK compliant #1738

Merged
merged 38 commits into from
Nov 14, 2023
Merged

Make cucumber CCK compliant #1738

merged 38 commits into from
Nov 14, 2023

Conversation

luke-hill
Copy link
Contributor

@luke-hill luke-hill commented Sep 11, 2023

Description

This PR aims to do 3 things

  • 1) make cucumber ruby v9.0.2+ CCKv12 CCKv13 (Thanks to bugfixes), compliant. We have let a lot slip by the wayside.
    • #attach now has a 3rd optional parameter filename which can provide a filename when renaming things
    • The TestStepResult message now has an additional 4th property of exception however in Ruby this is a partial duplication. This is definitely backwards compatible as we are not changing the existing error message property
    • The ParameterType message now has an additional 6th property of source_reference which is a location object returning a URI (File location), and line (Line number), of where the parameter types transformer is located
      • We need to amend messages to expose the transformer property (And tidy up some slightly inconsistent properties), on the ParameterType object during creation
  • 2) Clear up a couple of tiny "nuances" that hide lots of issues when running local vs remote. The primary one is CCK tests should always be runnable. This way we don't let things slip again
  • 3) Remove a couple of CI fixes for travis. We don't use travis

Type of change

Please delete options that are not relevant.

  • Refactoring (improvements to code design or tooling that don't change behaviour)
  • New features
  • Bug fix (non-breaking change which fixes an issue)

Changelog entry needs doing after everything has been merged.

This fixes a bunch of issues of interdependency hell. Everything "should" now be permissible with the latest versions of internals.

Checklist:

Your PR is ready for review once the following checklist is
complete. You can also add some checks if you want to.

  • Tests have been added for any changes to behaviour of the code
  • New and existing tests are passing locally and on CI
  • bundle exec rubocop reports no offenses
  • RDoc comments have been updated
  • CHANGELOG.md has been updated

@luke-hill luke-hill force-pushed the dependency_bump branch 2 times, most recently from 26609c5 to a9c3259 Compare September 15, 2023 15:15
@luke-hill
Copy link
Contributor Author

luke-hill commented Sep 18, 2023

@botandrose - You're not needed here. But posting here so you have tracking capabilities.

My next few priorities (in order of timeline pref)

  1. Fix this (I know who needs to help)
  2. Release a v9.0.3 cucumber ruby. EDIT: < Unsure what version this will be now. As we're making a few changes. Might be a 9.1
  3. Wait 1-2 weeks to see if nothing breaks
  4. Go back to other items such as your core update, get those reviewed and merged
  5. Do 1-2 weeks of other refactoring / work
  6. Release cucumber core v13

This probably takes me close to end of 2023

@botandrose
Copy link
Contributor

@luke-hill looks like you've got your work cut out for you! If you want to delegate some of that out to me, I enjoy refactoring and etc, and I'd be happy to throw some hours at it if it results in a sooner v13!

@luke-hill
Copy link
Contributor Author

If you want to tackle refactoring e.t.c. feel free. But the timeline needs to be this way because the v12 cucumber core is a chunky amount of refactoring and then v13 will have a new breaking change for a boolean param.

The issue is the cucumber ruby repo is tracking an old cck version, and I've never been involved with the cck physically. So I need someone to coach me on how it works and how to fix the problem (We're currently 2 majors behind the cck spec and the cck is likely to be updated soon as well).

@luke-hill
Copy link
Contributor Author

@botandrose heads up this is likely to take a LOT longer than initially anticipated.

Consequently I might do some more refactoring / rubocop fixes as this will be several weeks when I get time to dive into this.

As such, if you want to also tackle tech debt / refactoring go for it.

@luke-hill luke-hill changed the title Re-conciliation of mis-configured versions Make cucumber CCKv12 compliant Oct 3, 2023
@luke-hill luke-hill changed the title Make cucumber CCKv12 compliant Make cucumber CCK compliant Oct 9, 2023
@luke-hill
Copy link
Contributor Author

luke-hill commented Oct 10, 2023

Note to self. CCKv13 is now primarily passing. CDATA issue is known about (Missing step def is now correctly flagged in error). FIXED

EDIT: 2nd note to self - Check recursive nature of messages being compared. Potentially something slipping through that shouldn't. FIXED

EDIT: 3rd note to self. CCK v13.0.1 should feature 2 bugfixes to ensure git branches aren't being detected. However this won't fix the issue with CDATA. FIXED

v13.0.2 of the CCK didn't get the cdata fix. So expect it in 13.0.3

@luke-hill
Copy link
Contributor Author

Note for watchers

cdata fix didn't make it into 13.0.2 of the CCK, but some more inconsistencies (Not failing ones), did.

13.0.2+ of the CCK is needed to make this PR green. ETA on getting all remaining reconciliation fixes is not yet known

@luke-hill
Copy link
Contributor Author

ping @botandrose - Made some decent headway in last 2-3 weeks. Now looking back at this PR as something that can be fixed up and completed soon

@davidjgoss davidjgoss self-requested a review November 10, 2023 21:11
@luke-hill luke-hill merged commit 4a603c0 into main Nov 14, 2023
15 checks passed
@luke-hill luke-hill deleted the dependency_bump branch November 14, 2023 09:07
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.

4 participants