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

Adding Input Parser to Alpha Branch #127

Merged
merged 9 commits into from Oct 2, 2018

Conversation

bgargi
Copy link
Collaborator

@bgargi bgargi commented Sep 21, 2018

  • Motivation : Porting the input parser along with unit tests to the alpha branch ( Issue #85)

  • Integration test yet to be added

@coveralls
Copy link

coveralls commented Sep 21, 2018

Coverage Status

Coverage increased (+1.3%) to 96.786% when pulling a512e23 on bgargi:ts-v1.0-alpha into 5f98d03 on prasadtalasila:ts-v1.0-alpha.

@bgargi bgargi changed the title Adding Input Parser Code and Unit Test Adding Input Parser to Alpha Branch Sep 21, 2018
@codecov
Copy link

codecov bot commented Sep 21, 2018

Codecov Report

Merging #127 into ts-v1.0-alpha will increase coverage by 1.29%.
The diff coverage is 100%.

Impacted file tree graph

@@                Coverage Diff                @@
##           ts-v1.0-alpha     #127      +/-   ##
=================================================
+ Coverage          95.48%   96.78%   +1.29%     
=================================================
  Files                 11       11              
  Lines                266      280      +14     
=================================================
+ Hits                 254      271      +17     
+ Misses                12        9       -3
Flag Coverage Δ
#integration 35.71% <0%> (-29.7%) ⬇️
#unit 62.14% <100%> (+6.87%) ⬆️
Impacted Files Coverage Δ
apps/network/lib/network/station/station.ex 100% <ø> (ø) ⬆️
apps/network/lib/network/station/FSM.ex 98.68% <ø> (ø) ⬆️
apps/network/lib/network/station/registry.ex 100% <100%> (+2.56%) ⬆️
apps/input_parser/lib/input_parser.ex 100% <100%> (ø)
apps/input_parser/lib/input_parser/application.ex 0% <0%> (-100%) ⬇️
apps/api/lib/api/application.ex 100% <0%> (+100%) ⬆️

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 5f98d03...a512e23. Read the comment docs.

@prasadtalasila
Copy link
Owner

prasadtalasila commented Sep 30, 2018

Suggestions:

  1. Add credo config and ignore specific false positives.
  2. Migrate itinerary unit tests from network app to util app.
  3. Upgrade exunit. Use setup and teardown features of unit tests for developing upcoming tests.
  4. Remove the dummy utl.ex file.
  5. Define behavior for input parser and update the input parser and its unit tests.

@prasadtalasila
Copy link
Owner

@bgargi
There is duplication of data in the PR. The following files exist in two locations.

 59,555 apps/input_parser/data/schedule.txt 
 59,555 data/schedule.txt 
 2,264 apps/input_parser/data/local_variables.txt 
 2,264 data/local_variables.txt 
 2,264 apps/input_parser/data/stations.txt 
 2,264 data/stations.txt 
 151 apps/input_parser/data/OMT.txt 
 151 data/OMT.txt 

Please keep the data in data/ directory at the top-level and remove the same from apps/input_parser/data. Do remember to update the references to files in the code base of input parser.

@prasadtalasila
Copy link
Owner

The code readability issues pointed out by credo are valid issues.

  Code Readability                                                              
┃ 
┃ [R] ↘ The alias `Util.Connection` is not alphabetically ordered among its 
┃       group.
┃       apps/util/test/unit/itinerary_test.exs:11:9 #(ItineraryTest)
┃ [R] ↘ The alias `Util.Connection` is not alphabetically ordered among its 
┃       group.
┃       apps/network/test/unit/collector_test.exs:11:9 #(CollectorTest)
┃ [R] ↘ The alias `Util.Connection` is not alphabetically ordered among its 
┃       group.
┃       apps/network/test/integration/station_stress_test.exs:10:9 #(StationStressTest)
┃ [R] ↘ The alias `Util.Connection` is not alphabetically ordered among its 
┃       group.
┃       apps/network/test/integration/station_processing_test.exs:13:9 #(StationProcessingTest)
┃ [R] ↘ The alias `Util.Connection` is not alphabetically ordered among its 
┃       group.
┃       apps/network/test/integration/station_correctness_test.exs:11:9 #(StationCorrectnessTest)
┃ [R] ↘ The alias `Util.Connection` is not alphabetically ordered among its 
┃       group.
┃       apps/network/test/integration/station_contract_test.exs:12:9 #(StationContractTest)
┃ [R] ↘ The alias `Util.Connection` is not alphabetically ordered among its 
┃       group.
┃       apps/network/test/integration/station_FSM_test.exs:12:9 #(StationFSMTest)

The logs are taken from travis build. Please rearrange the alias lines into correct alphabetical order.

@prasadtalasila prasadtalasila merged commit 9fa2449 into prasadtalasila:ts-v1.0-alpha Oct 2, 2018
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