xtf-cz / xtf

MIT License
12 stars 54 forks source link

XTF is missing test coverage #325

Open rsvoboda opened 4 years ago

rsvoboda commented 4 years ago

XTF is missing test coverage

I haven't find any tests. Please prove me wrong.

dsimansk commented 4 years ago

That's absolutely correct, there're none. Can you share any expectation for minimal acceptable test coverage?

rsvoboda commented 4 years ago

I expect test coverage for key functionalities so people have guidance for the usage

mnovak1 commented 4 years ago

Missing test coverage is one of the biggest painpoints of XTF framework. Currently there is basically no way how to provide reasonable guarantees that backward compatibility is not broken and there are no regressions which makes upgrades to new XTF version expensive as time is burned on fixing issues.

As a start test should be provided to test API of main classes: OpenShifts OpenShift Waiters Waiter Image BuildManager Https

as those seem to be heavily used.

mnovak1 commented 4 years ago

Any ideas how the test suite could be run against real Openshift instance as part of PR review process?

Looks like that kubernetes-client/openshift-client project is mocking Openshift server. For example see test [1]. It might be tricky to use it for XTF as it does not have to reflect changes in API model in new Openshift versions or reflect precisely current Openshift API.

[1] https://github.com/fabric8io/kubernetes-client/blob/master/kubernetes-tests/src/test/java/io/fabric8/openshift/client/server/mock/OpenshiftRoleTest.java

mnovak1 commented 4 years ago

I've started to evaluate possibility to use Openshift CI (https://steps.svc.ci.openshift.org/help) for verification of PRs during dev review. admins/reviewers would have rights to trigger XTF TS against real OCP 4 instance. Local TS runs will need its own OCP 4 instance. From my pov it's better than mocking Openshift as it reflects changes in OCP 4 API. Drawback is that TS will take longer to run (in comparison to mocking) and there is no OCP 3 instance in Openshift CI for testing of backward compatibility. But it's better than current status and currently best solution I've found.

Any objections to this way?

mchoma commented 4 years ago

sounds good

fabiobrz commented 2 years ago

Here I am @mnovak1 - after closing #476. I've gone through the comments and noticed that https://steps.svc.ci.openshift.org/help is dead. IMHO we need to find some more reliable solution in case we don't want to use the mock.

BTW - using the mock is a quick and easy way to start and build the internal TS. Conversely, thinking about testing against an actual OCP backend would involve more steps and complexity (BTW this is demonstrated that the URL is dead after two years since the last comment).

I'd vote for starting from the bottom, baby steps, and add simple tests that use the mock. Which is there already. WDYT? CC @mchoma

simkam commented 2 years ago

yeah, URL with help changed but OpenShift CI is still well alive and more mature, see https://docs.ci.openshift.org/docs/

fabiobrz commented 2 years ago

yeah, URL with help changed but OpenShift CI is still well alive and more mature, see https://docs.ci.openshift.org/docs/

Nice to know that, thanks @simkam WDYT about the above options then? i.e. building an automation system that uses OpenShift CI vs. start by steps and add some tests that use a mock? AFAICT the first option is more complex and involves several activities compared to start coding some tests that use the mock (although those could just be smoke tests maybe).

mnovak1 commented 2 years ago

There are pros/cons for both approaches. Hopefully we might use both of them. Starting directly with real OCP seems to be challenging for CI, those tests are time consuming and take more time to develop. So we can start with mocking but it would great to have a way to switch those tests against real OCP instance. For example when testing backward compatibility with older OCP clusters.

fabiobrz commented 2 years ago

There are pros/cons for both approaches. Hopefully we might use both of them. Starting directly with real OCP seems to be challenging for CI, those tests are time consuming and take more time to develop. So we can start with mocking but it would great to have a way to switch those tests against real OCP instance. For example when testing backward compatibility with older OCP clusters.

+1, I am exactly on the same page, thanks @mnovak1 - do you want for me to file a child issue that I could use to prepare some basic mock-based test just to start with a baseline?

simkam commented 2 years ago

Mock server appears to be simple web server. It can return/store some (yaml) resources. It cannot do any operation like builds, etc. It might be useful for some unit tests, but not for integration testing. I'm skeptical about possibility to use a single test for running against both, the mock server and OpenShift cluster.

fabiobrz commented 2 years ago

Mock server appears to be simple web server. It can return/store some (yaml) resources. It cannot do any operation like builds, etc. It might be useful for some unit tests, but not for integration testing. I'm skeptical about possibility to use a single test for running against both, the mock server and OpenShift cluster.

Hi @simkam - Can we start by some baseline implementation? Simple unit tests to see how the components that XTF provides work against a mock. Why not?

IMHO there are at least some valid reasons for moving on like this:

WDYT?

simkam commented 2 years ago

Hi @simkam - Can we start by some baseline implementation? Simple unit tests to see how the components that XTF provides work against a mock.

sure