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

Allow comparison of two report directories #1635

Merged
merged 1 commit into from
Aug 7, 2018

Conversation

csordasmarton
Copy link
Contributor

No description provided.

@csordasmarton csordasmarton added enhancement 🌟 CLI 💻 Related to the command-line interface, such as the cmd, store, etc. commands test ☑️ Adding or refactoring tests labels Jun 8, 2018
@csordasmarton csordasmarton added this to the release 6.8 milestone Jun 8, 2018
@csordasmarton csordasmarton requested a review from gyorb June 8, 2018 10:00
Copy link
Contributor

@gyorb gyorb left a comment

Choose a reason for hiding this comment

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

Please extend the documentation and the help messages that the base name and the new name can be local directories too.

for res in baseline_dir_results:
if res.report_hash not in new_hashes:
filtered_reports.append(res)
print(base_hashes)
Copy link
Contributor

Choose a reason for hiding this comment

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

I think these prints were left from debugging.

@csordasmarton csordasmarton force-pushed the compare-report-directories branch from 3bd65be to 9fd7d97 Compare July 3, 2018 11:31
@csordasmarton csordasmarton requested a review from gyorb July 3, 2018 11:32
@Ericsson Ericsson deleted a comment Jul 3, 2018
@Ericsson Ericsson deleted a comment Jul 3, 2018
@Ericsson Ericsson deleted a comment Jul 3, 2018
@Ericsson Ericsson deleted a comment Jul 3, 2018
@Ericsson Ericsson deleted a comment Jul 3, 2018
@csordasmarton csordasmarton force-pushed the compare-report-directories branch from 9fd7d97 to 355de6d Compare July 3, 2018 11:39
@Ericsson Ericsson deleted a comment Jul 3, 2018
@Ericsson Ericsson deleted a comment Jul 3, 2018
@Ericsson Ericsson deleted a comment Jul 3, 2018
@Ericsson Ericsson deleted a comment Jul 3, 2018
Copy link
Contributor

@gyorb gyorb left a comment

Choose a reason for hiding this comment

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

It would be nice to print out the whole path or product url and run name at the end of the comparison output. So the user can see what was compared.
If there is a local report directory with the same name as a run the local directory will be used for comparison and not the remote run at the server.

@csordasmarton csordasmarton force-pushed the compare-report-directories branch 2 times, most recently from 533aa6d to b139ca6 Compare July 11, 2018 14:15
@csordasmarton
Copy link
Contributor Author

@gyorb Done. I have also refactored the code a little bit. Now we are handling cases when basename parameter is a report directory and newname is a remote run. I have also added test cases to this.

@csordasmarton csordasmarton force-pushed the compare-report-directories branch from b139ca6 to a574a2c Compare July 11, 2018 14:17
@Ericsson Ericsson deleted a comment Jul 11, 2018
@csordasmarton csordasmarton force-pushed the compare-report-directories branch from a574a2c to 249ae60 Compare July 12, 2018 07:23
@Ericsson Ericsson deleted a comment Jul 12, 2018
@Ericsson Ericsson deleted a comment Jul 12, 2018
@Ericsson Ericsson deleted a comment Jul 12, 2018
@Ericsson Ericsson deleted a comment Jul 12, 2018
@csordasmarton csordasmarton force-pushed the compare-report-directories branch from 249ae60 to c1167dd Compare July 12, 2018 07:28
@Ericsson Ericsson deleted a comment Jul 12, 2018
@Ericsson Ericsson deleted a comment Jul 12, 2018
@Ericsson Ericsson deleted a comment Jul 12, 2018
@csordasmarton csordasmarton requested a review from gyorb July 12, 2018 07:41
@dkrupp dkrupp requested review from lorincbalog, bruntib and dkrupp July 12, 2018 13:23
@csordasmarton csordasmarton force-pushed the compare-report-directories branch from c1167dd to f12a4c1 Compare July 30, 2018 08:26
@Ericsson Ericsson deleted a comment Jul 30, 2018
args.basename,
os.path.abspath(args.newname))
print_diff_results(reports)
LOG.info("Compared remote run %s and local report directory %s",
Copy link
Contributor

Choose a reason for hiding this comment

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

remote run can be multiple runs if a user gave a wildcard pattern like run*, the printed text should reflect that.
The same issue applies to other places where run names are used for comparison.

It could help if we would print out the matching run names in these cases.

* It is possible to compare two report directory.
* Handle cases when basename is a report directory and newname
is a remote run.
* Filter the results of local comparison.
* Test cases for local comparison.
@csordasmarton csordasmarton force-pushed the compare-report-directories branch from f12a4c1 to 3eb925f Compare August 1, 2018 07:39
@csordasmarton csordasmarton requested a review from gyorb August 1, 2018 07:40
@Ericsson Ericsson deleted a comment Aug 1, 2018
@gyorb gyorb merged commit 6f477bc into Ericsson:master Aug 7, 2018
@csordasmarton csordasmarton deleted the compare-report-directories branch August 14, 2018 09:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLI 💻 Related to the command-line interface, such as the cmd, store, etc. commands enhancement 🌟 test ☑️ Adding or refactoring tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants