search

konveyor / go-konveyor-tests

Apache License 2.0
1 stars 14 forks source link

[RFR] Update dependency check #221

Open nachandr opened 1 week ago

nachandr commented 1 week ago

The following scenario is currently not checked . This PR addresses the missing scenario - When dependencies are not specified in the test and the API request response returns dependencies .

Before fix, When dependencies are not specified in the test and the API request response returns dependencies , the test passed.

After fix, When dependencies are not specified in the test and the API request response returns dependencies , the test fails.

Note: An analysis test will still pass when dependencies are not specified in the test and if the API response request returns an empty list for dependencies. The fix doesn't change this behavior.

aufi commented 1 week ago

IIUC the aim of this PR is to not allow pass analysis test without specifying dependencies.

image

I don't think this is something we want. Optional skip of Dependencies or Tags in test assertion (defined by the test case), looks as the right option to me.

For visiblity @mguetta1 @jmle

mguetta1 commented 1 week ago

IIUC the aim of this PR is to not allow pass analysis test without specifying dependencies.

image

I don't think this is something we want. Optional skip of Dependencies or Tags in test assertion (defined by the test case), looks as the right option to me.

For visiblity @mguetta1 @jmle

I saw the need for such change after that Igor reported this regression bug: https://issues.redhat.com/browse/MTA-4169 At that time I wondered if we were seeing it in the API tests as well and realized that deps check was skipped for this app

nachandr commented 6 days ago

IIUC the aim of this PR is to not allow pass analysis test without specifying dependencies.

image

I don't think this is something we want. Optional skip of Dependencies or Tags in test assertion (defined by the test case), looks as the right option to me.

For visiblity @mguetta1 @jmle

Hi @aufi , Thanks for reviewing the PR. I noticed that when dependencies(or tags) are not specified for a test and when the API request response returns dependencies (or tags), the test still passes . The code changes address this scenario.

I'd like to clarify that an analysis test will still pass when dependencies (or tags) are not specified in the test iff the API response request returns the dependency/tag count as zero. For eg: The tier 2 run for this PR shows that the Apache Wicket analysis passed (This test doesn't have dependencies listed) Update_dependency

aufi commented 6 days ago

Understood, the question is if we need more assert empty dependencies list or ability to not check dependencies (in this "easy" way), deffering to others. @mguetta1 @sshveta

mguetta1 commented 6 days ago

Understood, the question is if we need more assert empty dependencies list or ability to not check dependencies (in this "easy" way), deffering to others. @mguetta1 @sshveta

To make it possible not to check dependencies in one case and check an expected empty list in other case, please see my suggestion above