BAEL-9084: Java API for GitHub using GitHub-API #18420
Conversation
libraries-5/src/test/java/com/baeldung/githubapi/UsersUnitTest.java
Outdated
Show resolved
Hide resolved
libraries-5/src/test/java/com/baeldung/githubapi/UsersUnitTest.java
Outdated
Show resolved
Hide resolved
| } | ||
|
|
||
| @Test | ||
| @Disabled // Needs credentials configuring |
There was a problem hiding this comment.
Can we specify how somewhere, maybe in a comment at the top of the file, so if a reader checks out the repo they understand what to do?
|
|
||
| import static org.junit.jupiter.api.Assertions.assertEquals; | ||
|
|
||
| public class ClientUnitTest { |
There was a problem hiding this comment.
If this is making a live connection it should be a LiveTest so it doesn't make the CI build flaky on connection issues
|
|
||
| import static org.junit.jupiter.api.Assertions.assertEquals; | ||
|
|
||
| public class UsersUnitTest { |
There was a problem hiding this comment.
If this is making a live connection it should be a LiveTest so it doesn't make the CI build flaky on connection issues
|
|
||
| GHUser user = gitHub.getUser("eugenp"); | ||
| List<GHRepository> repositoriesList = user.listRepositories().toList(); | ||
| assertEquals(6, repositoriesList.size()); |
There was a problem hiding this comment.
These kind of assertions will not be stable (can't rely on it always being 6)
There was a problem hiding this comment.
Is it better to just change this to a log then? Or is it ok to have unstable assertions if I change it to a LiveTest?
| String branchHash = branch.getSHA1(); | ||
|
|
||
| GHCommit commit = repository.getCommit(branchHash); | ||
| System.out.println(commit.getCommitShortInfo().getMessage()); |
There was a problem hiding this comment.
We have guidlines not to use System.out.println - use an SLF4J logger instead
There was a problem hiding this comment.
I even knew that and still forgot to do it. :( Fixing.
| assertEquals("someone", myself.getLogin()); | ||
| assertEquals("[email protected]", myself.getEmail()); | ||
| assertEquals(50, myself.getFollows().size()); |
There was a problem hiding this comment.
Confused about this could ever have passed?
There was a problem hiding this comment.
It wouldn't. This was just an example. It's not really possible to have stable assertions for this test, since it relies on knowing which user is authenticating.
I can just remove it if that's preferred? Or would it be better to just change the assertions to logs?
Co-authored-by: Liam Williams <[email protected]>
Co-authored-by: Liam Williams <[email protected]>
….java Co-authored-by: Liam Williams <[email protected]>
….java Co-authored-by: Liam Williams <[email protected]>
|
|
||
| GHUser user = gitHub.getUser("eugenp"); | ||
| List<GHRepository> repositoriesList = user.listRepositories().toList(); | ||
| assertTrue(repositoriesList.size() > 0); |
There was a problem hiding this comment.
We have the convention/guideline to use AssertJ for assertions like this, e.g.assertThat(..).isNotEmpty()
No description provided.