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

Make port range larger in test #3059

Merged
merged 1 commit into from
Mar 9, 2022
Merged

Conversation

mortalisk
Copy link
Contributor

@mortalisk mortalisk commented Mar 8, 2022

Test was not working when many workers running, as no ports were available.
https://ci.equinor.com/scout/job/komodo_testing/job/ert-custom-script-pr/9340/console

@@ -50,8 +50,9 @@ def _run(

def evaluate(
ensemble: Ensemble,
custom_port_range: Optional[range] = None,
Copy link
Contributor

@xjules xjules Mar 8, 2022

Choose a reason for hiding this comment

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

Are we to expose port_range as parameter here? It smells a bit, what do you think @jondequinor ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I dunno. Any reason why not?

Copy link
Contributor

Choose a reason for hiding this comment

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

You might be right though as I can't see any other way to deliver the port range there.

Copy link
Contributor

Choose a reason for hiding this comment

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

I agree with both of you.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

🐒 🐒 🐒 🐒 ❓

@codecov-commenter
Copy link

codecov-commenter commented Mar 8, 2022

Codecov Report

Merging #3059 (762f2b7) into main (6b59105) will increase coverage by 0.00%.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##             main    #3059   +/-   ##
=======================================
  Coverage   65.47%   65.48%           
=======================================
  Files         641      641           
  Lines       50631    50631           
  Branches     4440     4440           
=======================================
+ Hits        33151    33155    +4     
+ Misses      15986    15984    -2     
+ Partials     1494     1492    -2     
Impacted Files Coverage Δ
ert_shared/ensemble_evaluator/config.py 95.40% <ø> (ø)
ert3/evaluator/_evaluator.py 96.96% <100.00%> (ø)
libres/lib/res_util/block_fs.cpp 54.11% <0.00%> (+0.29%) ⬆️
ert_gui/simulation/run_dialog.py 77.03% <0.00%> (+0.70%) ⬆️

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 6b59105...762f2b7. Read the comment docs.

Copy link
Contributor

@xjules xjules left a comment

Choose a reason for hiding this comment

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

🚀

@mortalisk mortalisk enabled auto-merge (rebase) March 9, 2022 11:16
@mortalisk mortalisk merged commit 4475cce into equinor:main Mar 9, 2022
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

4 participants