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

Improve test functionality - SALEOR_215 #5255

Merged
merged 7 commits into from
Nov 18, 2024
Merged

Conversation

michalina-graczyk
Copy link
Member

@michalina-graczyk michalina-graczyk commented Nov 15, 2024

What type of PR is this?

  • 💅 Refactor
  • 🌟 Feature
  • 🔥 Bug Fix
  • 🔩 Maintenance
  • 🛠 Workflow CI/CD changes

Related Issues or Documents

  • closes #

Usage Instructions, Screenshots, Recordings

Have you written tests?

  • Yes!
  • No... here is why: Writing tests are mandatory, please replace this text with why test are not included in this PR

[Optional] Description

Copy link

changeset-bot bot commented Nov 15, 2024

🦋 Changeset detected

Latest commit: 2904632

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
saleor-dashboard Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@michalina-graczyk michalina-graczyk marked this pull request as ready for review November 15, 2024 11:08
@michalina-graczyk michalina-graczyk requested review from a team as code owners November 15, 2024 11:08
@github-actions github-actions bot temporarily deployed to pr-5255 November 15, 2024 11:08 Destroyed
Copy link
Member

@andrzejewsky andrzejewsky left a comment

Choose a reason for hiding this comment

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

OK, fix only the changeset

Comment on lines +349 to +354
const calculateDiscountedPrice = (
undiscountedPrice: number,
discountPercentage: number,
): number => {
return undiscountedPrice - (undiscountedPrice * discountPercentage) / 100;
};
Copy link
Member

Choose a reason for hiding this comment

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

This is dangerous: you just create a code to test the code. It implements the way discount is being calculated, meaning the test now is aware of it while it should not. Everywhere when possible make assertions on real values, because the incoming data is always known (you know what prices given product has because it's test data)

Non, blocking for now, but please keep in mind that.

"saleor-dashboard": patch
---

Improve inline discount test precision and reliability in draft orders
Copy link
Member

Choose a reason for hiding this comment

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

Please write changeset where you clearly explain what you have improved. Something like

Now, test that checks discount calculation asserts against fixed precision (2 decimals)

@andrzejewsky andrzejewsky self-requested a review November 15, 2024 12:54
@github-actions github-actions bot temporarily deployed to pr-5255 November 15, 2024 12:58 Destroyed
@michalina-graczyk michalina-graczyk changed the title Improve test functionality Improve test functionality - SALEOR_215 Nov 15, 2024
andrzejewsky
andrzejewsky previously approved these changes Nov 15, 2024
szczecha
szczecha previously approved these changes Nov 15, 2024
@github-actions github-actions bot temporarily deployed to pr-5255 November 15, 2024 15:02 Destroyed
szczecha
szczecha previously approved these changes Nov 15, 2024
andrzejewsky
andrzejewsky previously approved these changes Nov 15, 2024
@github-actions github-actions bot temporarily deployed to pr-5255 November 18, 2024 10:29 Destroyed
@andrzejewsky andrzejewsky self-requested a review November 18, 2024 14:01
@andrzejewsky andrzejewsky merged commit 4dac4f2 into main Nov 18, 2024
13 of 15 checks passed
@andrzejewsky andrzejewsky deleted the MERX-fix-SALEOR_215 branch November 18, 2024 14:18
poulch pushed a commit that referenced this pull request Nov 19, 2024
* Improve test functionality

* add changset

* improved changeset

* Add more details to changeset

* remove unnecessery line

* empty commit
michalina-graczyk added a commit that referenced this pull request Nov 22, 2024
* Improve test functionality

* add changset

* improved changeset

* Add more details to changeset

* remove unnecessery line

* empty commit
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants