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 racc test more flexible (for JRuby). #194

Merged
merged 1 commit into from
Nov 10, 2022

Conversation

enebo
Copy link
Contributor

@enebo enebo commented Nov 10, 2022

JRuby uses these same files for testing racc. The existing logic will not find 'racc' in a JRuby project checkout. This change allows it to work by just assuming 'ruby -S racc' when running tests. This will not change C Ruby's detection when setting up tests (since earlier checks will find racc).

JRuby uses these same files for testing racc. The existing logic will not
find 'racc' in a JRuby project checkout. This change allows it to work by
just assuming 'ruby -S racc' when running tests. This will not change C
Ruby's detection when setting up tests (since earlier checks will find
racc).
@enebo
Copy link
Contributor Author

enebo commented Nov 10, 2022

I should add that this gem will run tests locally as part of CI but once we have merged the gem into JRuby proper (similar to MRI) we will run these are part of the larger test suite and that is where this logic cannot find racc.

@flavorjones
Copy link
Collaborator

LGTM, thanks for the upstream contrib.

@flavorjones flavorjones merged commit f3c551c into ruby:master Nov 10, 2022
@flavorjones
Copy link
Collaborator

@enebo Do you need a patch release made for this?

@enebo
Copy link
Contributor Author

enebo commented Nov 11, 2022

@flavorjones from a basic cleanliness standpoint it would be good but I locally committed that file to our repo for now so it is not causing any red atm. So if not a lot of problems sure. If a pain then it can wait :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants