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.

Sign up for GitHub

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

Jump to bottom

[FEAT] Add support for pushing the amplification results in a MongoDB database #850

Closed
wants to merge 11 commits into from
Closed

[FEAT] Add support for pushing the amplification results in a MongoDB database #850

wants to merge 11 commits into from

Conversation

henry-lp
Copy link
Contributor

@henry-lp henry-lp commented Jul 28, 2019 •
edited
Loading

Hi , related issue (#843) . Ready for merging after done reviewing and change requests :).

@coveralls
Copy link

coveralls commented Jul 28, 2019 •
edited
Loading

Pull Request Test Coverage Report for Build 2139

  • 151 of 155 (97.42%) changed or added relevant lines in 6 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.4%) to 84.68%
Changes Missing Coverage Covered Lines Changed/Added Lines %
dspot/src/main/java/eu/stamp_project/mongodb/MongodbManager.java 85 89 95.51%
Totals Coverage Status
Change from base Build 2107: 0.4%
Covered Lines: 5074
Relevant Lines: 5992
💛 - Coveralls

@henry-lp henry-lp mentioned this pull request Jul 28, 2019
Open
@danglotb
Copy link
Member

danglotb commented Jul 29, 2019 •
edited
Loading

The changes look good.

Could you create a dedicated method for updating the database? i.e. not doing all the database stuff in reportStd, since this method should only output on the standard output.

@henry-lp
Copy link
Contributor Author

henry-lp commented Jul 29, 2019 •
edited
Loading

Hi @danglotb :), you mean an update method which only update an already existed document document ? . If that's the case then they already have this updateOne or updateMany method which take in a filter query and a new document . More details here. Basicly it would look like this, if you was to update one document matching with the provided query.

mongoClient = connectToMongo(this.mongoUrl);
MongoDatabase database = getDatabase(this.dbName,mongoClient);
MongoCollection<Document> coll = getCollection(this.colName,database);
coll.updateOne(QUERY,new Document("$set", NEWDOC));

Sorry if I missunderstood something , please point out if that's the case :) .

danglotb
danglotb reviewed Jul 29, 2019
View reviewed changes
dspot/src/main/java/eu/stamp_project/dspot/selector/PitMutantScoreSelector.java Outdated
@@ -235,6 +239,14 @@ public TestSelectorElementReport report() {
}

private String reportStdout() {
if (MongodbManager.getInstance().getDbConnectable()) {
Copy link
Member

Choose a reason for hiding this comment

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

Hi @tailp,

I would like the following if block in a dedicated method, like reportMongoDB().

Basically, create dedicated methods for all actions that are done on the mongoDB, in order to separate the mongoDB management from the rest of the code.

Copy link
Contributor Author

@henry-lp henry-lp Jul 29, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

ah I think I understand your point :). I made some changes just now. Hopefully this passes the tests on the CI and is what you wanted.

Copy link
Contributor Author

@henry-lp henry-lp Aug 1, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

hi @danglotb, I put all codeblocks into reportMongoDB() functions :) in the latest commit. I would be nice if you have some spare time to check if it's ok to go or if some more fixes I need to do. Thank you for your patience.

@monperrus
Copy link
Member

Per our discussion today, we will split this PR in two

  • the first one to introduce a DspotInformationCollector infrastructure
  • the second one to introduce the optional dependency to MongoDB

monperrus
monperrus reviewed Jul 30, 2019
View reviewed changes
dspot/src/main/java/eu/stamp_project/dspot/selector/JacocoCoverageSelector.java Outdated
@@ -222,6 +223,10 @@ public TestSelectorElementReport report() {
.append("%")
.append(AmplificationHelper.LINE_SEPARATOR);
lastReport = new TestSelectorElementReportImpl(report.toString(), jsonReport(coverageResults));

// Sending data to Mongodb
MongodbManager.getInstance().reportJacocoMongoDb(this.currentClassTestToBeAmplified.getQualifiedName(),"" + this.initialCoverage.getInstructionsCovered(),"" + coverageResults.getInstructionsCovered(),"" + coverageResults.getInstructionsTotal());
Copy link
Member

@monperrus monperrus Jul 30, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

collector.reportJacocotInformation()

monperrus
monperrus reviewed Jul 30, 2019
View reviewed changes
dspot/src/main/java/eu/stamp_project/dspot/selector/PitMutantScoreSelector.java Outdated
@@ -235,6 +236,8 @@ public TestSelectorElementReport report() {
}

private String reportStdout() {
MongodbManager.getInstance().reportPitMutantMongoDB(this.currentClassTestToBeAmplified.getQualifiedName(),"" + this.originalKilledMutants.size(),"" + this.getNbTotalNewMutantKilled());
Copy link
Member

Choose a reason for hiding this comment

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

collector.reportMutantInformation()

monperrus
monperrus reviewed Jul 30, 2019
View reviewed changes
dspot/src/main/java/eu/stamp_project/Main.java
@@ -59,6 +60,10 @@ public static void run() {
// global report handling
Main.GLOBAL_REPORT.output();
Main.GLOBAL_REPORT.reset();
// Send info to Mongodb
if (MongodbManager.getInstance().getDbConnectable()) {
Copy link
Member

Choose a reason for hiding this comment

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

collector.reportInitInformation()

@henry-lp
Copy link
Contributor Author

henry-lp commented Aug 6, 2019

Will open a new PR for this :) .

@henry-lp henry-lp closed this Aug 6, 2019
@henry-lp henry-lp mentioned this pull request Aug 6, 2019
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@danglotb danglotb danglotb left review comments

@monperrus monperrus monperrus left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@henry-lp @coveralls @danglotb @monperrus