This repository was archived by the owner on Mar 24, 2026. It is now read-only.
[ci fw-only Go/revel] Repair revel tests, make future failures less punishing#2625
Merged
michaelhixson merged 2 commits intoTechEmpower:masterfrom Mar 28, 2017
Merged
Conversation
Recent changes to the revel framework broke our revel tests. (Our code seems to depend on whatever is in the master branch of revel's github repository at the moment that we run the tests, so I guess problems like this are inevitable.) Specifically, the changes that broke us are: - controller.RenderJson was renamed to RenderJSON - controller.RenderArgs was renamed to ViewArgs revel/revel@6ac1603 revel/revel@0e454f2
The "revel run" command compiles the test code and launches the application server. If the test code has compilation errors, then the command fails and does not launch the server. If the command is run in the foreground, then a failure will be noticed by our toolset and the test will terminate in a timely fashion. If the command is run in the background, however, then a failure will not be noticed and our toolset will enter a loop, waiting for the application server port to be bound. In the failure case, the port is never bound and the toolset ends up waiting for 105 minutes before finally giving up. In the success case, whether the application runs in the foreground or background makes no difference. It's ok for the setup script to never terminate as long as it successfully launches the application server. The toolset will eventually notice that it binds to the expected port and will then run the appropriate benchmarks. The 105 minute timeout seems extremely large, but the reason it's that large is that there is at least one framework (wt) can take on the order of one hour to compile.
|
Thanks @michaelhixson for contributing to The Framework Benchmarks! @robfig, @methane and @msmith-techempower, code you've worked on has been modified. If you have the chance, please review. If you wish to unsubscribe from these notices, please open a Pull Request with the commit message |
methane
approved these changes
Mar 27, 2017
Contributor
|
@michaelhixson Could you add |
michaelhixson
changed the title
Contributor
Author
|
@methane Thanks for looking at this.
That seems like a good idea, but I'm going to leave that for someone else to implement. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
A couple of function/property names have changed in the revel framework, causing our test code to not compile. I updated our code to use the new names.
Also, the fact that
setup.shwas doing compilation in the background meant the failures went unnoticed by our toolset (specifically byframework_test.py), causing a long period of inactivity in the test suite. Removing the&fixes that problem, which will be useful in case the revel framework ever changes in a way that breaks our test code again.There are more details in the commit messages.