-
Notifications
You must be signed in to change notification settings - Fork 13
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
Add new demo env that uses faked execution server #187
Conversation
55b720a
to
40a09e9
Compare
demo/fake_server.ex
Outdated
def start_execution(execution_id, group_id, targets, env, config \\ []) do | ||
create_fake_execution(execution_id, group_id, targets) | ||
Process.sleep(2_000) | ||
complete_fake_execution(execution_id, group_id, targets, :passing) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we randomize the result with https://hexdocs.pm/elixir/1.12/Enum.html#random/1
You can skip the function parameter and do it directly there
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@fabriziosestito @rtorrero In our offline conversation, i just was saying, that we can have this execution result coming from the checks results, which now I see is fairly simple. It doesn't need to be random.
You just need to loop through the check results, check the result of them, and simply weight them (or some basic operation to find the worst check result).
Anyway, I'm fine on improving this on future iterations
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I find the current implementation too dump honestly.
We need to think on having certain logic like:
If the expectations are true -> the result of the check is true, etc
We cannot have failed expectations and passing results.
Now that we will implement the new view of the results using all these data, it would look totally illogical.
I think that we need to invest much more on this.
If you want to do in a subsequent PR is still good.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @rtorrero
I still think that there are unfinished things, but I'm fine merging this first to at least enable the demo environment, and we can work on it later at some point
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM.
Some few typo,copy/paste and linting errors.
You need to add the demo
folder to the .formatter.exs
and .credo.exs
file, to be included in the checks
27d8cd0
to
be28e2f
Compare
Initial implementation of a faker to generate data for the checks execution.
It achieves this by providing a implementation for the execution server that instead of triggering executions on agents uses the factories from the tests to generate such results.