- 
                Notifications
    You must be signed in to change notification settings 
- Fork 2.8k
          Eliminate network dependencies in podman search e2e tests with mock registry
          #27333
        
          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
base: main
Are you sure you want to change the base?
Conversation
1188b17    to
    a5e82e5      
    Compare
  
    | [NON-BLOCKING] Packit jobs failed. @containers/packit-build please check. Everyone else, feel free to ignore. | 
        
          
                test/e2e/search_test.go
              
                Outdated
          
        
      | search := podmanTest.Podman([]string{"search", "quay.io/podman/stable"}) | ||
| search.WaitWithDefaultTimeout() | ||
| Expect(search).Should(ExitCleanly()) | ||
| registryAddress, srv, serverErr := CreateMockRegistryServer() | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can this be done in a BeforeEach?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, I have changed the test to use BeforeEach and AfterEach.
        
          
                test/e2e/search_mock_registry.go
              
                Outdated
          
        
      | contentTypeJSON = "application/json" | ||
| ) | ||
|  | ||
| var searchResults = map[string]interface{}{ | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
s/interface{}/any everywhere please
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please squash these changes into the proper commit where you added them, appending them into the last commit makes it harder to review commit for commit and isn't right, the interface{} syntax should not be introduced to begin with.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay, I fixed it.
        
          
                test/e2e/search_test.go
              
                Outdated
          
        
      | Expect(search).To(ExitWithError(125, "getting repository tags: fetching tags list: StatusCode: 404")) | ||
| Expect(search.OutputToStringArray()).To(BeEmpty()) | ||
| }) | ||
| Describe("podman search do not crash on public registries", func() { | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should really not do this anymore, any public dependency will bite us. You now make us depend on at least three different registries which can still fail if the registry is down.
At the very least this should be skipped on PRs but I rather not do this at all as part of the normal CI.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay, I'll remove it.
de5401c    to
    49fc8ab      
    Compare
  
    49fc8ab    to
    fcb24aa      
    Compare
  
    - Add comprehensive mock registry server for e2e search tests - Replace quay.io and other external registry calls with local mock - Improve test reliability by removing network dependencies - Maintain full test coverage with controlled mock data Fixes: containers#27304 Fixes: https://issues.redhat.com/browse/RUN-3623 Signed-off-by: Jan Rodák <[email protected]>
…leanly Signed-off-by: Jan Rodák <[email protected]>
Signed-off-by: Jan Rodák <[email protected]>
fcb24aa    to
    af2d913      
    Compare
  
    There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks
| [APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: Honny1, Luap99 The full list of commands accepted by this bot can be found here. The pull request process is described here 
Needs approval from an approver in each of these files:
 
 Approvers can indicate their approval by writing  | 
This PR refactors the
podman searche2e tests to eliminate external network dependencies and improve test reliability by introducing a comprehensive mock registry server.Fixes: #27304
Fixes: https://issues.redhat.com/browse/RUN-3623
Does this PR introduce a user-facing change?