Skip to content

fix:Temporarily disable Discount code POST test to fix build #4854

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Jun 5, 2018
Merged

fix:Temporarily disable Discount code POST test to fix build #4854

merged 1 commit into from
Jun 5, 2018

Conversation

@dr0pdb
Copy link
Contributor

@dr0pdb dr0pdb commented Jun 5, 2018

Fixes #4853

Checklist

  • I have read the Contribution & Best practices Guide and my PR follows them.
  • My branch is up-to-date with the Upstream development branch.
  • The unit tests pass locally with my changes
  • I have added tests that prove my fix is effective or that my feature works
  • I have added necessary documentation (if appropriate)
  • All the functions created/modified in this PR contain relevant docstrings.

Short description of what this resolves:

currently the build is failing due to an unknown error arising due to the discount code post request.

Changes proposed in this pull request:

For now, skipping the test untill we figure out the problem so that it doesn't hamper the progress of the project.

@ghost ghost added the needs-review label Jun 5, 2018
@ghost ghost assigned dr0pdb Jun 5, 2018
@codecov
Copy link

codecov bot commented Jun 5, 2018 •
edited
Loading

Codecov Report

❗ No coverage uploaded for pull request base (development@ca73045). Click here to learn what that means.
The diff coverage is n/a.

Impacted file tree graph

@@              Coverage Diff              @@
##             development   #4854   +/-   ##
=============================================
  Coverage               ?   65.2%           
=============================================
  Files                  ?     213           
  Lines                  ?    9848           
  Branches               ?       0           
=============================================
  Hits                   ?    6421           
  Misses                 ?    3427           
  Partials               ?       0

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ca73045...ea6fbf3. Read the comment docs.

@dr0pdb
Copy link
Contributor Author

dr0pdb commented Jun 5, 2018 •
edited
Loading

@niranjan94 @SaptakS @iamareebjamal Please review and merge.
I understand that this is a workaround and isn't the recommended way but I think we shouldn't let this issue halt the project's development.

Kreijstal
Kreijstal reviewed Jun 5, 2018
View reviewed changes
Copy link
Member

@Kreijstal Kreijstal left a comment

Choose a reason for hiding this comment

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

Why didn't you just comment it out?

@dr0pdb
Copy link
Contributor Author

dr0pdb commented Jun 5, 2018

@Kreijstal Yeah, didn't think of it. Thanks for adding it.
I am squashing this though since we don't need two commits for it.

Kreijstal
Kreijstal previously approved these changes Jun 5, 2018
View reviewed changes
@ghost ghost removed the needs-review label Jun 5, 2018
@Kreijstal Kreijstal mentioned this pull request Jun 5, 2018
Closed
Kreijstal
Kreijstal previously approved these changes Jun 5, 2018
View reviewed changes
nzec
nzec previously approved these changes Jun 5, 2018
View reviewed changes
iamareebjamal
iamareebjamal reviewed Jun 5, 2018
View reviewed changes
tests/hook_main.py
event = EventFactoryBasic()
db.session.add(event)
db.session.commit()
transaction['skip'] = True
Copy link
Member

@iamareebjamal iamareebjamal Jun 5, 2018

Choose a reason for hiding this comment

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

Add a TODO comment so it it searchable in future

Copy link
Contributor Author

@dr0pdb dr0pdb Jun 5, 2018

Choose a reason for hiding this comment

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

done.

@dr0pdb dr0pdb dismissed stale reviews from nzec and Kreijstal via ea6fbf3 June 5, 2018 07:08
schedutron
schedutron approved these changes Jun 5, 2018
View reviewed changes
Copy link
Member

@schedutron schedutron left a comment

Choose a reason for hiding this comment

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

Seems good for now.

schedutron
schedutron reviewed Jun 5, 2018
View reviewed changes
tests/hook_main.py
event = EventFactoryBasic()
db.session.add(event)
db.session.commit()
transaction['skip'] = True
Copy link
Member

@schedutron schedutron Jun 5, 2018

Choose a reason for hiding this comment

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

What does transaction['skip'] = True do?

Copy link
Contributor Author

@dr0pdb dr0pdb Jun 5, 2018

Choose a reason for hiding this comment

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

skips the test

@fossasia fossasia deleted a comment Jun 5, 2018
@fossasia fossasia deleted a comment Jun 5, 2018
@fossasia fossasia deleted a comment Jun 5, 2018
@bhaveshAn bhaveshAn merged commit a36dc2c into fossasia:development Jun 5, 2018
@ghost ghost added the ready-to-ship label Jun 5, 2018
@dr0pdb dr0pdb deleted the temp_build_solution branch June 5, 2018 09:16
@Kreijstal Kreijstal mentioned this pull request Jun 7, 2018
Closed
@iamareebjamal iamareebjamal mentioned this pull request May 15, 2020
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@iamareebjamal iamareebjamal iamareebjamal approved these changes

+4 more reviewers

@schedutron schedutron schedutron approved these changes

@bhaveshAn bhaveshAn bhaveshAn approved these changes

@Kreijstal Kreijstal Kreijstal left review comments

@nzec nzec nzec left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

@dr0pdb dr0pdb

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Temporarily disable Discount code post test

6 participants

@dr0pdb @Kreijstal @iamareebjamal @nzec @schedutron @bhaveshAn