Experimental conformance suite: Extended skipped tests not being reported

[sunjayBhatia]

With a conformance suite config similar to the below:

	cSuite, err := suite.NewExperimentalConformanceTestSuite(suite.ExperimentalConformanceOptions{
		Options: suite.Options{
                        ...
                        // Note this vs. using the EnableAllSupportedFeatures and ExemptFeatures field
			SupportedFeatures: suite.AllFeatures.Delete(suite.MeshCoreFeatures.UnsortedList()...),
			SkipTests: []string{
				tests.HTTPRouteRedirectPortAndScheme.ShortName,
			},
		},
		Implementation: conformance_v1alpha1.Implementation{
                         ...
		},
		ConformanceProfiles: sets.New(
			suite.HTTPConformanceProfileName,
			suite.TLSConformanceProfileName,
		),
	})

The resulting conformance report does not show any tests are skipped:

        apiVersion: gateway.networking.k8s.io/v1alpha1
        date: "2023-07-19T20:34:17Z"
        gatewayAPIVersion: TODO
        implementation:
          contact:
          - '@projectcontour/maintainers'
          organization: projectcontour
          project: contour
          url: https://github.com/projectcontour/contour
          version: ""
        kind: ConformanceReport
        profiles:
        - core:
            result: success
            statistics:
              Failed: 0
              Passed: 10
              Skipped: 0
            summary: ""
          name: TLS
        - core:
            result: success
            statistics:
              Failed: 0
              Passed: 28
              Skipped: 0
            summary: ""
          extended:
            result: success
            statistics:
              Failed: 0
              Passed: 9
              Skipped: 0
            summary: ""
            supportedFeatures:
            - HTTPRouteQueryParamMatching
            - HTTPRouteSchemeRedirect
            - HTTPRouteHostRewrite
            - HTTPRoutePathRewrite
            - HTTPRouteRequestMirror
            - HTTPResponseHeaderModification
            - HTTPRoutePortRedirect
            - HTTPRoutePathRedirect
            - HTTPRouteMethodMatching
          name: HTTP

[slayer321]

Hey @shaneutt, I did like to work on this issue. Will need some pointers how can I start working on it.

[shaneutt]

I'm sorry I made a mistake with this one. I accepted it but now in retrospect as I look at it again, I think this was intended behavior @sunjayBhatia: the idea being that extended tests are optional, so you don't "opt out" of them, you only "opt in". :thinking:

I would be curious to hear what other conformance reviewers and approvers think about making this change?

/cc @arkodg @mlavacca @michaelbeaumont @LiorLieberman @Xunzhuo

[arkodg]

feels like a bug, if a test from a feature suite is being skipped, the statistics.Skipped should reflect that, for all profile types

[LiorLieberman]

feels like a bug, if a test from a feature suite is being skipped, the statistics.Skipped should reflect that, for all profile types

+1, If someone opted in to a a feature suit, everything that is skipped should be reflected imho.

However, in this specific case, we need to define what happens with Mesh features, because the opt in is to AllFeatures but we delete the mesh features.

I see it as - whatever feature that is in the SupportedFeatures set and is skipped should be included in the Skipped field

[mlavacca]

feels like a bug, if a test from a feature suite is being skipped, the statistics.Skipped should reflect that, for all profile types

+1, I think that the skipped value should be incremented if a test is skipped, even for extended features. Additionally, the SkippedTests field should contain all the skipped tests.

[shaneutt]

Ok sounds good, thanks for the input.

Please feel free to get started on this one @slayer321, and thank you.

/assign @slayer321

If you have any questions or get stuck, please do drop questions in here. :vulcan_salute:

[sunjayBhatia]

agree with the above comments 👍🏽

[slayer321]

Hey @shaneutt , I was looking into this issue below is what I have observe.

• At first I run the ExperimentalConformance test for contour with above details that are provided by sunjay. Where he is skipping HTTPRouteRedirectPortAndScheme . After checking the internal working of the report generation I found that here is where this test is just not get added to the conformance profile because it has SupportGatewayPort8080 as the supportedFeature and when the check is run it makes hasAllFeatures variable as false and this HTTPRouteRedirectPortAndScheme test don't get added to conformance profile . After checking the HTTPConformanceProfile we can see the Core supports SupportGateway,SupportReferenceGrant, SupportHTTPRoute and ExtendedFeatures supports HTTPRouteExtendedFeatures , and both of them don't include SupportGatewayPort8080 so that's the reason check failed.
• I tried adding some other test to skip i.e HTTPRouteRequestMirror and it works as this is available in HTTPRouteExtendedFeatures

extended:
  result: partial
  skippedTests:
  - HTTPRouteRequestMirror
  statistics:
    Failed: 0
    Passed: 8
    Skipped: 1

• One way I found that adding HTTPRouteRedirectPortAndScheme to skip get's it added to the report if we add the SupportGatewayPort8080 to the HTTPConformanceProfile CoreFeature .. below is what I get the output after running the test

extended:
  result: partial
  skippedTests:
  - HTTPRouteRequestMirror
  - HTTPRouteRedirectPortAndScheme
  statistics:
    Failed: 0
    Passed: 8
    Skipped: 2

I'm not sure if this is the solution we are looking for please let me know your thoughts on this.

[shaneutt]

@slayer321 your issue seems to be distinct from the issue of how we report non-executed extended tests, could you please create a separate new issue for this problem, with reproduction step and file this as a bug? Thank you!

[k8s-triage-robot]

This issue has not been updated in over 1 year, and should be re-triaged.

You can:

• Confirm that this issue is still relevant with /triage accepted (org members only)
• Close this issue with /close

For more details on the triage process, see https://www.kubernetes.dev/docs/guide/issue-triage/

/remove-triage accepted

[k8s-triage-robot]

The Kubernetes project currently lacks enough contributors to adequately respond to all issues.

This bot triages un-triaged issues according to the following rules:

• After 90d of inactivity, lifecycle/stale is applied
• After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
• After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

• Mark this issue as fresh with /remove-lifecycle stale
• Close this issue with /close
• Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle stale

[k8s-triage-robot]

The Kubernetes project currently lacks enough active contributors to adequately respond to all issues.

This bot triages un-triaged issues according to the following rules:

• After 90d of inactivity, lifecycle/stale is applied
• After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
• After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

• Mark this issue as fresh with /remove-lifecycle rotten
• Close this issue with /close
• Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle rotten

[candita]

/remove-lifecycle rotten

[mlavacca]

/area conformance-machinery

[k8s-triage-robot]

The Kubernetes project currently lacks enough contributors to adequately respond to all issues.

This bot triages un-triaged issues according to the following rules:

• After 90d of inactivity, lifecycle/stale is applied
• After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
• After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

• Mark this issue as fresh with /remove-lifecycle stale
• Close this issue with /close
• Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle stale

[k8s-triage-robot]

The Kubernetes project currently lacks enough active contributors to adequately respond to all issues.

This bot triages un-triaged issues according to the following rules:

• After 90d of inactivity, lifecycle/stale is applied
• After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
• After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

• Mark this issue as fresh with /remove-lifecycle rotten
• Close this issue with /close
• Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle rotten