Skip to content

Conversation

@methane
Copy link
Contributor

@methane methane commented Jul 2, 2014

When I run tests, there is only one result in results.json.
It was because benchmarker makes child process and results updated in child process.

@methane methane changed the title Fix results.json only contains result of last test. Fix results.json contains only result of last test. Jul 2, 2014
@methane methane changed the title Fix results.json contains only result of last test. [toolset] Fix results.json contains only result of last test. Jul 3, 2014
@hamiltont
Copy link
Contributor

Just experienced this issue and I can confirm that using these commits fixed the problem. My test results now show up in both results.json files

Copy link
Contributor

Choose a reason for hiding this comment

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

For what it's worth, this line creates potential to experience UnboundLocalVariable, which I just experienced by trying to add log.debug("Before load %s" % str(results)). Not sure if there's a way to ensure this doesn't happen to anyone else?

Copy link
Contributor

Choose a reason for hiding this comment

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

Perhaps you could use a local variable named something other than results?

Copy link
Contributor

Choose a reason for hiding this comment

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

Ok, so my python knowledge isn't deep. I could easily fix this by using self.results instead of trying to print the results variable. However, it may still be useful to rename that local variable just to avoid any future problems

Copy link
Contributor Author

Choose a reason for hiding this comment

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

else: clause of try: statement will be executed only when try: block finished without any exception.
But it's not easy to understand for person from other languages.

Copy link
Contributor

Choose a reason for hiding this comment

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

There is nothing wrong with the logic here, I'm saying that perhaps it's not a good choice to have a variable self.results and a variable results, they are easily confused. Perhaps rename results

@jyentechempower jyentechempower merged commit 199a901 into TechEmpower:master Jul 17, 2014
@methane methane deleted the fix-one-result branch July 18, 2014 03:40
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.

3 participants