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

Fix fakeredis count system #114

Merged
merged 3 commits into from
Jan 25, 2021

Conversation

alain-andre
Copy link
Contributor

Fix error due to fakeredis and use a real one like other tests. It fixes also some errors like about status in returned json (not job[status]) and quotas mistake.

@fab-girard, @braktar we were not agree where to put the count_increase for an optimization, this still can be moved. Do we want to increase the count every time a VRP is valid or only when a full result is given ?

The only errors and failures are now due to known build_detail method, rubocop (@senhalil), a random time test and array that should be empty.

@senhalil
Copy link
Contributor

@alain-andre I don't know why you tagged me for rubocop?

Rubocop rules are changed in PR #68 and it is merged recently without fixing the new errors due changed rules. I am not surprised that it fails.

@alain-andre
Copy link
Contributor Author

@alain-andre I don't know why you tagged me for rubocop?

Rubocop rules are changed in PR #68 and it is merged recently without fixing the new errors due changed rules. I am not surprised that it fails.

Ok, I did not notice this, I still had in mind that fixing rubocop to v1.81 was enough not to break tests. I'll check.

@senhalil
Copy link
Contributor

Don't know about the rubocop version, I don't remember when I touched it last time. However, I know that two days ago when I created PR #111 and pushed commit 3a696e9 rubocop test was okay (because I have a git-hook which verifies this) and it still is okay at that moment in time.

(system-3.6.9) halil@:~/dev/optimizer-api((HEAD detached at 3a696e9a))$ git checkout 3a696e9a2b0d56d5469759da82752732b784068f
HEAD is now at 3a696e9a Fix: eliminate negatif infeasible quantities
post-checkout hook is called
(system-3.6.9) halil@:~/dev/optimizer-api((HEAD detached at 3a696e9a))$ optim_test_rubocop 
Started with run options --verbose --seed 58700

  /0: [=---=---=---=---=---=---=---=---=---=---=---=---=---=---=---=---=---=---=] 0% Time: 00:00:00,  ETA: ??:??:??

RubocopTest#test_no_offenses_in_tests
  1/1: [========================================================================] 100% Time: 00:00:01, Time: 00:00:01

Finished in 1.05855s
1 tests, 1 assertions, 0 failures, 0 errors, 0 skips

Rakefile Outdated Show resolved Hide resolved
@alain-andre alain-andre force-pushed the fix-fakeredis-count-system branch 3 times, most recently from a41c54f to 43e0881 Compare January 25, 2021 10:59
@fab-girard fab-girard self-requested a review January 25, 2021 12:32
test/api/v01/vrp_test.rb Show resolved Hide resolved
test/api/v01/vrp_test.rb Show resolved Hide resolved
@senhalil
Copy link
Contributor

If I do bundle install in this branch Gemfile.lock changes.. @alain-andre is this the case if you do bundle install on your side? If that is the case, can you push that version of the Gemfile.lock?

@alain-andre
Copy link
Contributor Author

If I do bundle install in this branch Gemfile.lock changes.. @alain-andre is this the case if you do bundle install on your side? If that is the case, can you push that version of the Gemfile.lock?

Yes you are right, I forgot to remove the gem limitations in lock file, I'm updating the commit.

@alain-andre alain-andre force-pushed the fix-fakeredis-count-system branch 2 times, most recently from 463620f to 5eb2a5c Compare January 25, 2021 16:18
Copy link
Contributor

@senhalil senhalil left a comment

Choose a reason for hiding this comment

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

white-space typos otherwise 👌

test/api/v01/vrp_test.rb Outdated Show resolved Hide resolved
test/api/v01/vrp_test.rb Outdated Show resolved Hide resolved
Co-authored-by: senhalil <halil.sen@mapotempo.com>
@fab-girard fab-girard merged commit 1b62ded into Mapotempo:dev Jan 25, 2021
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