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

e2e: simplify e2e test script #133

Merged
merged 3 commits into from
Aug 19, 2024
Merged

Conversation

wbpcode
Copy link
Contributor

@wbpcode wbpcode commented Aug 15, 2024

  1. Fix the naming. In previous implementation, the provider.cc actually is consumer and the consumer.cc actually is provider.
  2. Merge two scenarios ([consumer -> provider] and [consumer -> bridge -> provider]) to one test.

Signed-off-by: WangBaiping <[email protected]>
@wbpcode
Copy link
Contributor Author

wbpcode commented Aug 16, 2024

This should also fixed CI problem in the #132

@wbpcode
Copy link
Contributor Author

wbpcode commented Aug 16, 2024

The coverage report was broken and we may need to remove it because the codecov only provide limited support to the free users.

@wbpcode
Copy link
Contributor Author

wbpcode commented Aug 19, 2024

friendly ping @wu-sheng

@wu-sheng
Copy link
Member

I can see you changed the CI names. Is that expected? And, there is one CI process fails.

@wu-sheng
Copy link
Member

I could change the required CIs, if those new names are expected.

@wbpcode
Copy link
Contributor Author

wbpcode commented Aug 19, 2024

yeah, the new names are expected.

@wu-sheng
Copy link
Member

New UT fail should be fixed?

@wbpcode
Copy link
Contributor Author

wbpcode commented Aug 19, 2024

I can see you changed the CI names. Is that expected? And, there is one CI process fails.

The coverage reporting is broken because the it's rate limited. /sad

@wu-sheng
Copy link
Member

How about we remove that to keep CI green?

@wbpcode
Copy link
Contributor Author

wbpcode commented Aug 19, 2024

New UT fail should be fixed?

I am inclined to remove the coverage reporting from the CI and add a simple coverage ratio checking in our CI directly with a new PR.

@wu-sheng
Copy link
Member

Please go ahead. Personally, I am not a fun about coverage rate.
Once codes are tested through CI, it is good. No matter how coverage says.

@wu-sheng
Copy link
Member

Should I merge first or wait foe your update?

@wbpcode
Copy link
Contributor Author

wbpcode commented Aug 19, 2024

If you don't care it's coverage ratio, then I can remove the failed coverage reporting directly in this PR. :)

@wbpcode
Copy link
Contributor Author

wbpcode commented Aug 19, 2024

I will push the new commit tonight

Signed-off-by: WangBaiping <[email protected]>
@wu-sheng wu-sheng merged commit c96eb92 into SkyAPM:main Aug 19, 2024
3 checks passed
@wbpcode wbpcode deleted the dev-simplify-e2e branch August 19, 2024 17:03
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.

2 participants