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

Fixing: Deprecate Using the yegor256/rultor Docker Image #1045

Closed
wants to merge 12 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 35 additions & 0 deletions src/test/java/com/rultor/agents/daemons/EndsDaemonITCase.java
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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)) {
Copy link
Contributor

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 :)

Copy link
Author

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?

Copy link
Contributor

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!

Copy link
Author

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?

final String dir = talk.read()
.xpath("/talk/daemon/dir/text()").get(0);
MatcherAssert.assertThat(
Copy link

Choose a reason for hiding this comment

The 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

Copy link
Author

Choose a reason for hiding this comment

The 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?

Copy link

@mkordas mkordas Mar 10, 2016 via email

Choose a reason for hiding this comment

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

Copy link
Author

Choose a reason for hiding this comment

The 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.

Copy link

@mkordas mkordas Mar 10, 2016 via email

Choose a reason for hiding this comment

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

dir,
StringContains.containsString(
"#### Deprecation Notice ####"
)
);
MatcherAssert.assertThat(
dir,
StringEndsWith.endsWith("##############")
);
}
}
}

/**
* Start a talk.
* @param talk Talk to start
Expand Down
100 changes: 72 additions & 28 deletions src/test/java/com/rultor/agents/daemons/StartsDaemonITCase.java
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -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);
Copy link

Choose a reason for hiding this comment

The 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

Copy link

Choose a reason for hiding this comment

The 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 ####")
Copy link

Choose a reason for hiding this comment

The 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?

Copy link
Author

Choose a reason for hiding this comment

The 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();
Expand Down Expand Up @@ -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;
}

}