-
Notifications
You must be signed in to change notification settings - Fork 156
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
Fixing: Deprecate Using the yegor256/rultor Docker Image #1045
Changes from all commits
a755cad
bcceb7f
01077c8
962c247
7af74de
172fca3
ea9cf66
9671aab
428f49c
1cfee31
038e253
929e881
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -39,7 +39,10 @@ | |
import org.apache.commons.io.FileUtils; | ||
import org.apache.commons.lang3.SystemUtils; | ||
import org.hamcrest.MatcherAssert; | ||
import org.hamcrest.core.StringContains; | ||
import org.hamcrest.core.StringEndsWith; | ||
import org.junit.Assume; | ||
import org.junit.Ignore; | ||
import org.junit.Rule; | ||
import org.junit.Test; | ||
import org.junit.rules.TemporaryFolder; | ||
|
@@ -102,6 +105,38 @@ public void readsExitCodeCorrectly() throws IOException { | |
); | ||
} | ||
|
||
/** | ||
* EndsDaemon can deprecate default image. | ||
* @throws IOException In case of error | ||
*/ | ||
@Test | ||
@Ignore | ||
public void deprecatesDefaultImage() throws IOException { | ||
final Talk talk = new Talk.InFile(); | ||
FileUtils.write( | ||
new File(this.start(talk, "").getAbsolutePath(), "testing"), "12" | ||
); | ||
new EndsDaemon().execute(talk); | ||
for (final String path | ||
: talk.read().xpath("/p/entry[@key='merge']/entry[@key='script']") | ||
) { | ||
if ("yegor256/rultor".equals(path)) { | ||
final String dir = talk.read() | ||
.xpath("/talk/daemon/dir/text()").get(0); | ||
MatcherAssert.assertThat( | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @sebing those assertions will still pass even if string will contain just one deprecation notice There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @mkordas Agree. But as long as we don't have an 'id' for each deprecation how to track it down? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
@sebing just assert that it occurs twice: at the end and at the beginning.
Or maybe make assertion for entire deprecation text.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @mkordas it occurs twice is a wrong test in my view. And I thought the actual message may vary and checking the content of a message is not a good idea. So I though it is safe checking the occurrence and it ends with the end of deprecation at the end of test. But if you think checking the whole message is okay(as of now we don't know the exact message) i can do that. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
@sebing OK, do let's leave just one assertion
|
||
dir, | ||
StringContains.containsString( | ||
"#### Deprecation Notice ####" | ||
) | ||
); | ||
MatcherAssert.assertThat( | ||
dir, | ||
StringEndsWith.endsWith("##############") | ||
); | ||
} | ||
} | ||
} | ||
|
||
/** | ||
* Start a talk. | ||
* @param talk Talk to start | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -33,6 +33,7 @@ | |
import com.jcabi.matchers.XhtmlMatchers; | ||
import com.jcabi.ssh.SSHD; | ||
import com.jcabi.ssh.Shell; | ||
import com.jcabi.xml.XML; | ||
import com.jcabi.xml.XMLDocument; | ||
import com.rultor.agents.shells.TalkShells; | ||
import com.rultor.spi.Agent; | ||
|
@@ -41,14 +42,17 @@ | |
import java.io.ByteArrayInputStream; | ||
import java.io.ByteArrayOutputStream; | ||
import java.io.File; | ||
import java.io.IOException; | ||
import java.io.InputStream; | ||
import java.util.concurrent.TimeUnit; | ||
import org.apache.commons.io.input.NullInputStream; | ||
import org.apache.commons.lang3.CharEncoding; | ||
import org.apache.commons.lang3.SystemUtils; | ||
import org.hamcrest.MatcherAssert; | ||
import org.hamcrest.Matchers; | ||
import org.hamcrest.core.StringStartsWith; | ||
import org.junit.Assume; | ||
import org.junit.Ignore; | ||
import org.junit.Rule; | ||
import org.junit.Test; | ||
import org.junit.rules.TemporaryFolder; | ||
|
@@ -81,6 +85,73 @@ public final class StartsDaemonITCase { | |
@Test | ||
public void startsDaemon() throws Exception { | ||
Assume.assumeFalse(SystemUtils.IS_OS_WINDOWS); | ||
final Talk talk = this.talk(); | ||
MatcherAssert.assertThat( | ||
talk.read(), | ||
XhtmlMatchers.hasXPaths( | ||
"/talk/daemon[started and dir]", | ||
"/talk/daemon[ends-with(started,'Z')]" | ||
) | ||
); | ||
final String dir = talk.read().xpath("/talk/daemon/dir/text()").get(0); | ||
final ByteArrayOutputStream baos = new ByteArrayOutputStream(); | ||
TimeUnit.SECONDS.sleep(2L); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @sebing please read http://www.kyleblaney.com/junit-best-practices/ - ideally we shouldn't have any sleeps in unit tests There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @sebing oh, sorry, you've just moved this code, please ignore above |
||
new Shell.Safe(new TalkShells(talk.read()).get()).exec( | ||
String.format("cat %s/stdout", dir), | ||
new NullInputStream(0L), | ||
baos, baos | ||
); | ||
MatcherAssert.assertThat( | ||
baos.toString(CharEncoding.UTF_8), | ||
Matchers.allOf( | ||
Matchers.containsString("+ set -o pipefail"), | ||
Matchers.containsString("+ date"), | ||
Matchers.containsString("+ ls -al"), | ||
Matchers.containsString("182f61268e6e6e6cd1a547be31fd8583") | ||
) | ||
); | ||
MatcherAssert.assertThat( | ||
new File(dir, "status").exists(), Matchers.is(false) | ||
); | ||
} | ||
|
||
/** | ||
* StartsDaemon can deprecate default image. | ||
* @throws IOException In case of error | ||
* @todo #1018:30min Implement a deprecation message at the start and end | ||
* of the process if the project is using the default image | ||
* 'yegor256/rultor'. Fix the issue using the xpath available in | ||
* com.rultor.agents.daemons.StartsDaemon and | ||
* com.rultor.agents.daemons.EndsDaemon. | ||
* @todo #1018:30min This test should not check for start and end | ||
* deprecation messages if when the repo is the actual Rultor repo: | ||
* https://github.com/yegor256/rultor | ||
*/ | ||
@Test | ||
@Ignore | ||
public void deprecatesDefaultImage() throws IOException { | ||
final Talk talk = this.talk(); | ||
final XML xml = talk.read(); | ||
for (final String path | ||
: xml.xpath("/p/entry[@key='merge']/entry[@key='script']") | ||
) { | ||
if ("yegor256/rultor".equals(path)) { | ||
final String dir = talk.read() | ||
.xpath("/talk/daemon/dir/text()").get(0); | ||
MatcherAssert.assertThat( | ||
dir, | ||
StringStartsWith.startsWith("#### Deprecation Notice ####") | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @sebing do we really expect this notice at the very beginning of this string? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @mkordas That is my understanding. |
||
); | ||
} | ||
} | ||
} | ||
|
||
/** | ||
* Creates a Talk object with basic parameters. | ||
* @return The basic Talk object for testing | ||
* @throws IOException In case of error | ||
*/ | ||
private Talk talk() throws IOException { | ||
final SSHD sshd = new SSHD(this.temp.newFolder()); | ||
final int port = sshd.port(); | ||
final Talk talk = new Talk.InFile(); | ||
|
@@ -116,33 +187,6 @@ public void startsDaemon() throws Exception { | |
).when(profile).assets(); | ||
final Agent agent = new StartsDaemon(profile); | ||
agent.execute(talk); | ||
MatcherAssert.assertThat( | ||
talk.read(), | ||
XhtmlMatchers.hasXPaths( | ||
"/talk/daemon[started and dir]", | ||
"/talk/daemon[ends-with(started,'Z')]" | ||
) | ||
); | ||
final String dir = talk.read().xpath("/talk/daemon/dir/text()").get(0); | ||
final ByteArrayOutputStream baos = new ByteArrayOutputStream(); | ||
TimeUnit.SECONDS.sleep(2L); | ||
new Shell.Safe(new TalkShells(talk.read()).get()).exec( | ||
String.format("cat %s/stdout", dir), | ||
new NullInputStream(0L), | ||
baos, baos | ||
); | ||
MatcherAssert.assertThat( | ||
baos.toString(CharEncoding.UTF_8), | ||
Matchers.allOf( | ||
Matchers.containsString("+ set -o pipefail"), | ||
Matchers.containsString("+ date"), | ||
Matchers.containsString("+ ls -al"), | ||
Matchers.containsString("182f61268e6e6e6cd1a547be31fd8583") | ||
) | ||
); | ||
MatcherAssert.assertThat( | ||
new File(dir, "status").exists(), Matchers.is(false) | ||
); | ||
return talk; | ||
} | ||
|
||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@sebing We need this to not fire when the repo is the actual Rultor repo: https://github.com/yegor256/rultor :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@original-brownbear Sorry to ask again ;)
How do I get repo of the current run? Is that information available in the xml or somehow I have to create
com.jcabi.github.Repo
?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@sebing puzzle is fine too ;) really you did a lot here already!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@original-brownbear I have added a puzzle. But for my curiosity, how do I know the repo?