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

[WIP] optimize code and add tests #22

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

krishna-parajuli
Copy link

No description provided.

@krishna-parajuli krishna-parajuli marked this pull request as ready for review April 21, 2024 03:33
@krishna-parajuli krishna-parajuli changed the title [WIP] Kp optimize code and add tests [WIP] optimize code and add tests Apr 21, 2024
@horrormyth
Copy link
Contributor

Since it's WIP, it'd be great to use the pytest, the project intends to use pytest for testing @krishna-parajuli , there is now pytest config as well.

@ashiishme
Copy link

@krishna-parajuli If possible, can you create a PR for the test configuration so that it can be merged and used by others?

@sapradhan
Copy link
Collaborator

@krishna-parajuli thank you for taking lead on writing tests but the changes so far seem to be very detailed unit tests. Can we focus on writing tests that test the functionality first? Please see my proposal in this comment - #11 (comment)

Please create a test framework that bootstraps running the code and makes the output available to test methods. We can then divide writing specific test cases per each requirement among other devs.

cc: @horrormyth

@horrormyth
Copy link
Contributor

@krishna-parajuli thank you for taking lead on writing tests but the changes so far seem to be very detailed unit tests. Can we focus on writing tests that test the functionality first? Please see my proposal in this comment - #11 (comment)

Please create a test framework that bootstraps running the code and makes the output available to test methods. We can then divide writing specific test cases per each requirement among other devs.

cc: @horrormyth

I agree with @sapradhan here, with a strong note that detailed unit tests for each function are necessary, the main reason being that these could be used elsewhere. We can argue that reusability is minimal in this project, but still, it would be good to go in that direction. Anyhow, for brevity, those functions could then be battle-tested from the main function that actually calls them with functionality/cases in mind. One advantage of this is that we don't necessarily need to provide expected fixtures for each of the functions; basically, whoever creates one writes the tests for it. The test sets of main/whatever_we_call should then cover all the intended test cases with the expected output that @sumanashrestha has mentioned and/or any corner cases. With that being said, and TLDR, it would be good to have the expected fixtures ready.

@samurato
Copy link
Contributor

samurato commented May 1, 2024

@horrormyth @ashiishme @krishna-parajuli @sapradhan I have raised a PR that tests the functionality of the program. here #89.
It would be nice to add a tests folder and add unit tests and other tests there so that every PR will do a Build and produce the report before merge for further request upto you guys.

tests
|---units
|--- smoke
|-- intgration 

Sample automated testing here.
https://github.com/samurato/center-randomize/actions/runs/8911951792/job/24474346754
its failing unit tests that ran 10 times as per the requirements student allocation per school in center has to be less than 200 .

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.

5 participants