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

Update Sort Function unit tests #87

Open
wants to merge 1 commit into
base: feature/sort-function-weighting
Choose a base branch
from

Conversation

jacksontaylor13
Copy link
Contributor

Since we are changing the concept of a SortFunction the unit tests have to change with it. This means that rather than comparing the timing we would compare the weights returned by the SortFunction.

However, this changes the concept of the unit tests entirely so we also need to add in unit tests for the TimingFunction's as well. To make sure that the refactor returns the same values, we would also need a combination of unit tests. This means the sort function would have to return the same values if we passed the weights into a standard timing function.

Since we are changing the concept of a `SortFunction` the unit tests have to change with it. This means that rather than comparing the timing we would compare the weights returned by the `SortFunction`.

However, this changes the concept of the unit tests entirely so we also need to add in unit tests for the `TimingFunction`'s as well. To make sure that the refactor returns the same values, we would also need a combination of unit tests. This means the sort function would have to return the same values if we passed the weights into a standard timing function.
@josmanperez
Copy link

Hi @jacksontaylor13 I am new to open source projects and collaborations, I've set a pull request for this project and I'm still waiting for the circle ci to passed.. the thing is that I've never used Circle CI nor know how to use it. Do I have to somehow pass the test on my own? or this is something that it supposed to be on the spruce part?

What I want to know if I have to do something about the circle ci process...

Thank you so much and sorry for this "silly" question but I'm kind of lost here :(

@jacksontaylor13
Copy link
Contributor Author

Hi @josmanperez! Not a silly question at all. Unfortunately I don't have control over the repo anymore but @mattyohe may be able to help you out. Running Circle CI is something that will be done from the Spruce Team so there should be nothing you need to worry about. Thank you for the contribution!

@eriklamanna
Copy link
Contributor

@josmanperez I have merged your changes and the new Swift 4 compatible 2.0 release is now available. Thanks again for your PR.

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

3 participants