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 partly #225. #228

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

marcemq
Copy link
Contributor

@marcemq marcemq commented Oct 18, 2019

What
Generate a final report for k8s-scaling.json result in different subdirectories which follows the current hard-coded test name k8s-scaling.

Edited files:

  • report/report_dockerfile/tidy_scaling.R
  • report/report_dockerfile/dut-details.R

Testing done:
Having the next results structure:

$ tree results/
results/
├── Env.R
├── scaling
│   └── k8s-scaling.json
└── scaling2
    └── k8s-scaling2.json

2 directories, 3 files

the k8s-scaling2.json does not follow the hard-coded test name, and only for it a warning message will be displayed in the final report, see report attached.
metrics_report.pdf

Generate a final report for k8s-scaling.json result in diferent
subdirectories which follows the current hard-coded test name
k8s-scaling.

Signed-off-by: Morales Quispe, Marcela <marcela.morales.quispe@intel.com>
@dklyle dklyle added the scaling label Oct 21, 2019
@grahamwhaley
Copy link

Ah, I meant to leave notes on the PR here, but left them on the Issue #225 instead - please take a look @marcemq

@grahamwhaley
Copy link

ping @marcemq - did you see the notes on #225 ? Make sense?
whilst here, I notice this PR and commit subject names are not very descriptive (Fix partly #225. ). Can you make it a meaningful short sentence. Have a look at the repo history for examples (git log --oneline for instance). thanks!

Note, btw, I see you did add a comprehensive set of info to the PR body - that afaik does not make it into the git log history itself - sometimes it is better to add, or duplicate, that information in the commit bodies, as that does make it into git.

@marcemq
Copy link
Contributor Author

marcemq commented Nov 13, 2019

@grahamwhaley I'll consider your comments about PR and git meaningful short sentence.

In terms of this fix and your comments at #225, I think we need to define a configurable report, so it works with this edge cases and the behaviour of the report phase is clearer and well define, I haven't had a chance to write it formally because right know I'm focusing on scale the test at #248.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants