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

Refactor git providers #438

Merged
merged 9 commits into from
Jul 14, 2021
Merged

Refactor git providers #438

merged 9 commits into from
Jul 14, 2021

Conversation

josecordaz
Copy link
Contributor

@josecordaz josecordaz commented Jul 7, 2021

What changed?

  • For go git integration tests we now can set an id to the cache used in a specific Describe section.
  • All git providers tests migrated to use ginkgo framework.

Why?

  • It was getting hard to maintain.
  • There were 2 bugs fixed when running this tests locally.
    -- The URL filter was not replacing values properly.
    -- When generating cache sometimes it was throwing Bad Credentials errors due to conflicts with other tests modifying GITHUB_TOKEN env variable.

How did you test it?

  • By rerunning all the tests locally and through Github actions.

- use a different cache set for each Describe section
- moved repo validation to the main function that creates it
- use h.GetAccountType anywhere
Other tests were changing the GITHUB_TOKEN env variable so when those tests run before git providers tests it was causing Bad Credentials errors when running locally.
@josecordaz josecordaz marked this pull request as ready for review July 7, 2021 20:12
Copy link
Contributor

@jrryjcksn jrryjcksn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

Would it be possible at some point to add a comment explaining how to generate the cache files?

@luizbafilho
Copy link
Contributor

Do we still need to manually edit the cache files after generate them?

@josecordaz
Copy link
Contributor Author

Do we still need to manually edit the cache files after generate them?

Sorry man, I'm confused. Why would we need to do something like that?. I probably know where the confusion is, though. Even though this runs as part of the unit tests job. I would like to see this as some sort of mix between unit tests and integration tests. Let's have a further discussion about this when I add documentation to clarify how this works in a follow up PR.

# Conflicts:
#	pkg/gitproviders/common_test.go
@josecordaz josecordaz merged commit 1a994fa into main Jul 14, 2021
@bigkevmcd bigkevmcd deleted the refactor-git-providers branch December 15, 2021 10:59
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.

None yet

4 participants