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 tests for survey response import #3578

Merged
merged 1 commit into from
Dec 8, 2021

Conversation

kael89
Copy link
Collaborator

@kael89 kael89 commented Dec 6, 2021

[No issue]

Not strictly required but in preparation for PHX-152, I felt that it would be worth refactoring the survey response import tests. Reasons:

  • I found it a bit hard to get an overview of the existing test cases
  • These tests use some older patterns. Refactoring to use newer setup and test data patterns should make them more maintainable

Changes overview

  1. Used semantic test ids (eg duplicate_quest_id1_test) instead of the existing, more realistic ids (eg fdfcc42a44705c032a8_test)
  2. Removed redundant test data (multiple responses testing the same logic, unnecessary models eg assign a Public permission group to a survey when permissions are not tested, etc)
  3. Extracted validation tests in their own file and parameterised them
  4. Updated the import spreadsheets according to the new changes + restructured them a bit

@kael89 kael89 force-pushed the refactor-import-response-tests branch from 7f6dca2 to 94eaeeb Compare December 6, 2021 21:42
Copy link
Contributor

@IgorNadj IgorNadj left a comment

Choose a reason for hiding this comment

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

Thanks for cleaning that up!

@kael89 kael89 merged commit 6f9d2c8 into dev Dec 8, 2021
@kael89 kael89 deleted the refactor-import-response-tests branch December 8, 2021 00:26
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