Skip to content

Conversation

@SoggyRhino
Copy link
Contributor

This pr is large but most of it isn't code just test data

Grade Loader

  • Updated to match loadProfiles in where it updates the global GradeMap instead of returning a map.
  • Updated to remove panic in favor of return errors
  • Moved io into csvToMap to fix defer in loop issue
  • minor clean up
  • Originally we were creating a log file for every csv, and this seemed pointless and also introduced a bug when test since the logs dir was hard coded as ./logs (testing uses a different working dir so ./logs did not exist). I thought this was unnecessary and so now it just logs the same as everywhere else (file and std out)

Tests

  • Pretty simple table based tests, basically just creating a test csv and then seeing if the output we read out what we expect
  • Used a couple different key types to make sure the map was keyed correctly

Profile Loader

  • simplified the json decoding

Test

  • Since the actual content of profiles.json isn't important we can just copy the professors.json (you need to remove sections and office hours but otherwise they are the same)
  • Then just load profiles.json and make sure that the professor maps are updated as we expect

Side effects

When I wrote the other tests gradeLoading was not working (due to above bug) so I needed to update tests to support it. Once the tests were fixed I realized that none of the test cases were for sections that actually had data so I added a new test and regenerated the testdata. This caused validator_test to break since the indexMap got messed due to the tests moving around. I ended up changing alot of validator_test to match the other tests and moved the loading data the TestMain.

I changed a lot and a good bit was based on personal preference so let me know if it needs any changes. It probably does but I am unable to find anything on my own.

- testdata
 - added a new case that had grade dist data

- gradeLoader.go
 - changed setting of GradeMap to match loadProfiles
 - added error return type
 - removed panic
 - removed seperated logging for grades
  - fixed bug where log dir was hard coded
 - changed loop to use os.ReadDir
 - csv to map
  - moved file io into (fix defer in loop issue)
  - added error return type/ remove panic
  - clean up

- main.go
 - changed mkdir to mkdir All for logging dir
 - use time.format instead of fmt
 - changed log panic on bad flags to log fatal

- methods.go
 - moved Unmarshall File from parser_test.go

- parser_test.go
 - moved testCourses,testSections and testProfessors from validator_test.go
 - added loading of above to TestMain
 - added gradeLoading
 - changed key for outputSections (fixed collision issue when parsing mulitple semesters)
 - added util functions FailTestIfNoPanic and FailTestIfPanic

- profileLoader.go
 - changed to use Unmarshall file / cleanup json decoding
 - added error return type

- sectionParser_test.go
  - updated to use new helper functions for panics

- validator.go
 - minor change to logging to remove \n

- validator_test.go
 - moved init and globals to parser_test.go
 - updated functions not use logs check success
 - updated functions that use indexMap to be work without it
  - this was causing tests to fail when /testdata what regenerated
 - added parallel calls

 - build.bat
  - gofmt formats recursively so we don't need ./... just ./
   - this was causing it to try to format other my other go projects
  - goimports is the same
# Conflicts:
#	parser/validator_test.go
updated docs
added test for loadgrades
rewrote test for profileLoader.go
updated -update
fixed bugs in the test,
@mikehquan19
Copy link
Contributor

Thank you! I will review them soon.

@SoggyRhino
Copy link
Contributor Author

SoggyRhino commented Sep 9, 2025

Just noticed that I set the update flag to default to true while debugging and forgot to change it back when I pushed it. Fixed in commit above.

@SoggyRhino
Copy link
Contributor Author

I'm going to close this since there are going to be large changes to the grade loader and profile loader, also I think there are areas that can be improved.

@SoggyRhino SoggyRhino closed this Oct 3, 2025
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