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

Proyecto terminado y funcional #20

Open
wants to merge 99 commits into
base: master
Choose a base branch
from
Open

Proyecto terminado y funcional #20

wants to merge 99 commits into from

Conversation

maurers
Copy link
Owner

@maurers maurers commented Nov 4, 2019

Agradeceré le den una revisada, estoy seguro que se pueden hacer mejoras, pero para efectos prácticos del ejercicio creo que se puede tomar por concluido salvo su mejor opinión.

@ArCiGo ArCiGo added enhancement New feature or request help wanted Extra attention is needed labels Nov 4, 2019
Copy link
Collaborator

@ArCiGo ArCiGo left a comment

Choose a reason for hiding this comment

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

Check the orthography and grammar of the assert messages

public class GetSpotifyTest extends BaseTest{
private String spotifyUrl;

@BeforeMethod
Copy link
Collaborator

Choose a reason for hiding this comment

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

Remove annotation and call the method inside of every test method

Copy link
Collaborator

@ArCiGo ArCiGo left a comment

Choose a reason for hiding this comment

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

Check the comments for the SignUpPageTest and GetSpotifyTest classes

spotifySignUpPage.clickOnRegistrateButton();
List<String> listOfErrors = spotifySignUpPage.getAllSpotifySignUpFormErrorMessages();

Assert.assertFalse(listOfErrors.isEmpty(), "No arrojo errores");
Copy link
Collaborator

Choose a reason for hiding this comment

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

You also need to verify the errors thrown by the form, not only the size of the list

@Test(description = "TC_US3_002/Campos de registro de obtén spotify gratis se envían llenos y sin validar captcha", alwaysRun = true, priority = 0)
public void spotifySignUpNoCaptcha(){

PropertyReader propertyReader = new PropertyReader();
Copy link
Collaborator

Choose a reason for hiding this comment

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

Declare the PropertyReader object as global

public void spotifySignUpFuturo(){

PropertyReader propertyReader = new PropertyReader();
String email=propertyReader.getProperty("credentials.properties", "TC_US_03_003_EMAIL");
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why are you repeating the same variables for each test method?

Create a list of user values where you can store the values from the credentials.properties file and call the list (myAwesomeList.get(0)) in the methods who receive the data.

@Test(alwaysRun = true,description = "TC_US4_001/ Llenar el formulario para crear cuenta nueva")
public void spotifySignNoGender(){
PropertyReader propertyReader = new PropertyReader();
String email=propertyReader.getProperty("credentials.properties", "TC_US_04_001_EMAIL");
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why are you repeating the same variables for each test method?

Create a list of user values where you can store the values from the credentials.properties file and call the list (myAwesomeList.get(0)) in the methods who receive the data.

private String spotifyUrl;

@BeforeMethod
public void initSetup(){
Copy link
Collaborator

Choose a reason for hiding this comment

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

Remove annotation and call the method inside of every test method

spotifySignUpPage.clickOnRegistrateButton();
List<String> listOfErrors = spotifySignUpPage.getAllSpotifySignUpFormErrorMessages();

Assert.assertFalse(listOfErrors.isEmpty(), "No arrojo errores");
Copy link
Collaborator

Choose a reason for hiding this comment

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

Check the grammar and orthography

private SpotifyHelpPage spotifyHelpPage;

@BeforeTest
public void initSetup() {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Remove annotation and call the method inside of every test method

driver.navigate().to(spotifyUrl);
spotifyHomePage = new SpotifyHomePage(driver);

Assert.assertTrue(spotifyHomePage.isLoaded(), "Página principal no se cargo correctamente");
Copy link
Collaborator

Choose a reason for hiding this comment

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

Check the grammar and orthography

spotifyHomePage.clickOnSpotifyIcon();

Assert.assertTrue(spotifyHomePage.isLoaded(), "Página principal no se cargo correctamente");
Assert.assertEquals(spotifyHomePage.BASE_URL, driver.getCurrentUrl(), "Cargó una página diferente al de home");
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think is not necessary to validate the current URL of the site. What if changes?


@BeforeTest
public void initSetup() {
spotifyUrl = propertyReader.getProperty("credentials.properties", "URL_WEBSITE");
Copy link
Collaborator

Choose a reason for hiding this comment

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

These values are not user credentials. Create a new properties file for these kinds of values

@BeforeTest
public void initSetup() {
spotifyUrl = propertyReader.getProperty("credentials.properties", "URL_WEBSITE");
helpUrl = propertyReader.getProperty("credentials.properties", "URL_HELP");
Copy link
Collaborator

Choose a reason for hiding this comment

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

Where are you using this variable?

import org.testng.annotations.Test;

public class SpotifyIconTest extends BaseTest {
private String spotifyUrl, helpUrl;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why these variables are global?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request help wanted Extra attention is needed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

9 participants