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

feat: add migrations check #196

Merged
merged 4 commits into from
May 25, 2023
Merged

Conversation

nsprenkle
Copy link
Contributor

@nsprenkle nsprenkle commented May 24, 2023

About

Adds a migrations check, mirrored after similar workflows in edx-platform, edx-notes-api, and edx-analytics-dashboard, to name a few.

Migration check further modified (based on this test) to actually fail when there are missing migration files.

JIRA: https://2u-internal.atlassian.net/browse/AU-820

Testing Instructions

  • Verify new step (Migrations check on mysql8) appears on PRs. Should fail if a migration fails or is missing for future PRs.

@nsprenkle nsprenkle marked this pull request as ready for review May 24, 2023 18:06
@codecov
Copy link

codecov bot commented May 24, 2023

Codecov Report

Patch coverage: 100.00% and no project coverage change.

Comparison is base (0962818) 93.43% compared to head (900111a) 93.43%.

Additional details and impacted files
@@           Coverage Diff           @@
##           master     #196   +/-   ##
=======================================
  Coverage   93.43%   93.43%           
=======================================
  Files          18       18           
  Lines        1996     1996           
  Branches      125      125           
=======================================
  Hits         1865     1865           
  Misses        118      118           
  Partials       13       13           
Flag Coverage Δ
unittests 93.43% <100.00%> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
submissions/__init__.py 100.00% <100.00%> (ø)

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@jansenk
Copy link
Contributor

jansenk commented May 24, 2023

I think that the check should be failing on this PR I made off of your branch: https://github.com/openedx/edx-submissions/pull/197/files

@jansenk
Copy link
Contributor

jansenk commented May 24, 2023

Wow, it looks like those aren't actually doing that check, it's just a unit test that's running?

openedx/edx-platform#32301

check migrations and check migrations mysql8 both pass! it's a unit test that fails, specifically this one.

@nsprenkle
Copy link
Contributor Author

nsprenkle commented May 25, 2023

Wow, it looks like those aren't actually doing that check, it's just a unit test that's running?

openedx/edx-platform#32301

check migrations and check migrations mysql8 both pass! it's a unit test that fails, specifically this one.

@jansenk, is that to say we should be doing that as a unit test, as in edx-platform? I'm happy to do it that way instead, but am just making educated guesses at this time.

Copy link
Contributor

@jansenk jansenk left a comment

Choose a reason for hiding this comment

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

@nsprenkle nsprenkle merged commit 14789dc into master May 25, 2023
@nsprenkle nsprenkle deleted the aurora/migration-check-workflow branch May 25, 2023 19:16
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.

2 participants