search

kiegroup / github-action-build-chain

85 stars 24 forks source link

[ISSUE 461] Fix issue when getting the fork name for the cases where the fork has a different name #462

Closed rgdoliveira closed 1 year ago

rgdoliveira commented 1 year ago

Fixes #461

Note: Please, do not check dist/index.js changes. It is automatically generated.

rgdoliveira commented 1 year ago

e2e tests are failing, I will investigate why...

jstastny-cz commented 1 year ago

@shubhbapna if you're available to discuss.

shubhbapna commented 1 year ago

@jstastny-cz You are correct, it should be sourceOwner (the e2e test will need to be fixed. it should probably just involve fixing the right API being mocked). As for removing this optimization, I would advice against it. We were getting rate limit errors almost every other day without this optimization (original PR: #411). Unless there has been a change in the naming convention for most of the forked repositories in apache for kiegroup projects and they don't follow the heuristic that we noticed in the originally.

rgdoliveira commented 1 year ago

@shubhbapna is it possible that you help us here related to the e2e tests please?

As part of this PR there is a commit fixing the e2e-regression tests that broke because some repos were migrated from kiegroup to Apache. (This one I think is correct)

And now I'm trying to fix the e2e tests, but I'm struggling with them. I fixed the single-pr.test.ts and just pushed a commit now but I'm not sure if this is actually the correct solution. There are still the full-downstream.test.ts and cross-pr.test.ts that are failing, so if could please help/advice. Thanks

rgdoliveira commented 1 year ago

So based on @shubhbapna comment, we followed the right path using sourceOwner when getting the fork name.

All the e2e tests passed no and I would like to ask @shubhbapna @jstastny-cz and @Ginxo to review the PR. Thanks!

rgdoliveira commented 1 year ago

@jstastny-cz there was already a test for the getForkName(), so I expanded it to also include what you suggested, please take a look.