People tend to not be aware of software testing patterns but the Page Object Model would help clean this code up a lot, especially around the hard element ids that could change at any time.

https://www.selenium.dev/documentation/test_practices/encour...

You're advocating to turn this:

public class Login {

  public void testLogin() {
    // fill login data on sign-in page
    driver.findElement(By.name("user_name")).sendKeys("userName");
    driver.findElement(By.name("password")).sendKeys("my supersecret password");
    driver.findElement(By.name("sign-in")).click();

    // verify h1 tag is "Hello userName" after login
    driver.findElement(By.tagName("h1")).isDisplayed();
    assertThat(driver.findElement(By.tagName("h1")).getText(), is("Hello userName"));
  }

}

Into this?:

public class SignInPage { protected WebDriver driver;

  // <input name="user_name" type="text" value="">
  private By usernameBy = By.name("user_name");
  // <input name="password" type="password" value="">
  private By passwordBy = By.name("password");
  // <input name="sign_in" type="submit" value="SignIn">
  private By signinBy = By.name("sign_in");

  public SignInPage(WebDriver driver){
    this.driver = driver;
  }

  /**
    * Login as valid user
    *
    * @param userName
    * @param password
    * @return HomePage object
    */
  public HomePage loginValidUser(String userName, String password) {
    driver.findElement(usernameBy).sendKeys(userName);
    driver.findElement(passwordBy).sendKeys(password);
    driver.findElement(signinBy).click();
    return new HomePage(driver);
  }

}

/** * Tests login feature */ public class TestLogin {

  @Test
  public void testLogin() {
    SignInPage signInPage = new SignInPage(driver);
    HomePage homePage = signInPage.loginValidUser("userName", "password");
    assertThat(homePage.getMessageText(), is("Hello userName"));
  }

}

There are better ways to write the pattern but the idea is having the elements in one place so if the site changes you can update them quickly, and then in your code accessing them by variable name. It's much cleaner, easier to edit if they change the UI (which happens) or if they use a web framework like React that has strange elements to access.

I don't know. I guess that's subjective. I would personally rather work on code with much less abstraction.

All of the Java codebases I've worked with, that have 20 levels of hierarchy to 'clean up' the code bases and make it easier to edit, turn into giant balls of spaghetti at some point.

I normally agree with code being subjective but with test code around the UI I'd argue actually that this is a good abstraction. For example this code would completely break if they added another div with text on a page. It'd also completely break the Page Object Model. But imagine there are 50 tests covering these pages. In the iterative example you'd need to update the divs for all 50 tests, with a page object model you'd need to update them in one place and the tests would then be updated. I appreciate this argument might come down to "trust me, I've seen this" but it really is a good abstraction to look at if you need to write selenium or cypress tests.

Selenium is admittedly not one of my major skillsets.. so I'll defer to your seemingly more knowledgable judgement on this. :)

Abstraction can be very useful, but it can also definitely lead to spaghetti if it gets out of hand or is done incorrectly.

It is? It looks pretty middle of the road to me. obvious issues:

1. undescriptive variable names (eg. data2)

2. inconsistent formatting

3. copy pasted fragments everywhere

I mean sure, it's not perfect. It might not get you through a FAANG interview, but it would get you through a Kellogg interview.

As someone who worked IT at Kellogg's, I would say the interview was more about confidence and communication skills then it was technical ability. The only "programming" I ever did was VBA/Excel. Although I did dabble with PLC from time to time. I'm not sure if they even have actual software engineers at their company, they seem to like contracting out a lot of that kind of stuff.

I wouldn't survive the interview because I would have to answer "Why do you want to work here?" with "To fund adult art commissions of Tony the Tiger."

I think a more commercially successful venture would be an adult cinematic franchise set in the Dark Frankenberry/Count Chocula universe as they save breakfast (and morning sex) from nefarious threats like the Frosted Lucky Charms Elf and the evil Quisp.

The M. Night Shamalayan twist....the Lucky Charms Elf ends up with the girl.

I don't think it would... A one minute review:

- Complicated flakey in-line xpaths '//*[@id="content"]/div/div[2]/div/div[1]/div[1]/div/div/button' ?!

- Time.sleep(x) is flakey, you should wait for the element to appear

- case first_name | last_name both uses a first_name() generator, should just be generate_name()

- Opening and closing the driver 10000 times

- Opening and closing the driver 10000 times

Can you be more clear on this? I don't see where this is happening. There's a start_drive function that they call once in main and use that reference the entire time.

https://github.com/SeanDaBlack/KelloggBot/blob/main/req.py#L...

- Complicated flakey in-line xpaths '//*[@id="content"]/div/div[2]/div/div[1]/div[1]/div/div/button'

How else would you get around navigating the DOM which this has to match precisely?

- case first_name | last_name both uses a first_name() generator, should just be generate_name()

Very subjective?

- Time.sleep(x) is flakey, you should wait for the element to appear.

Agree with this but maybe it's not possible for some reason in regards to how the webpage is configured?

> Can you be more clear on this? I don't see where this is happening. There's a start_drive function that they call once in main and use that reference the entire time.

https://github.com/SeanDaBlack/KelloggBot/blob/main/req.py#L...

They run start_driver() inside a while loop with 10000 iterations

> How else would you get around navigating the DOM which this has to match precisely?

For that specific one I'd probably try "//*[contains(text(), 'Apply now')]", if they add a single element to that page the entire xpath will fail in the code example

> Very subjective?

Do you have two first names?

> Agree with this but maybe it's not possible for some reason in regards to how the webpage is configured?

Maybe! Still, I bet you it's possible to wait for an element even if it's your own action taking effect.

> They run start_driver() inside a while loop with 10000 iterations

I mean wouldn't that allow 10,000 parallel operations? If you waited for each one to complete, then close each one, it would take forever.

EDIT: Gruez(below) made the point that the code isn't written to be async so this is definite issue.

> For that specific one I'd probably try "//*[contains(text(), 'Apply now')]", if they add a single element to that page the entire xpath will fail in the code example

You're prob right about this to be honest.

However, there's two elements with 'Apply now' on the page. A button with 'Apply now' and a dropdown with 'Apply now' that opens when you click that button. Lol

https://jobs.kellogg.com/job/Lancaster-Permanent-Production-...

Welcome to scraping hell!

> Do you have two first names?

It's generating fake names. It doesn't matter.

> However, there's two elements with 'Apply now' on the page. A button with 'Apply now' and a dropdown with 'Apply now' that opens when you click that button. Lol

You can chain them! It's still better to [0] the text one and then click the pop-up than use div chains the whole way down. Yes it's not perfect, if I was testing the site as part of Kellogg I'd ask for test ids to make them unique, but it's still immune to additional divs.

> It's generating fake names. It doesn't matter.

This hurts my soul

Jeff Jeff and John John are frequent applicants at Kellogg for some reason.

>Can you be more clear on this? I don't see where this is happening. There's a start_drive function that they call once in main and use that reference the entire time.

>https://github.com/SeanDaBlack/KelloggBot/blob/main/req.py#L...

the call to "start_driver" is inside the "while (i < 10000)" loop

I mean wouldn't that allow 10,000 parallel operations? If you waited for each one to complete, then close each one, it would take forever.

Theoretically it can be rewritten to do the requests in parallel, but currently it isn't.

If it was you don't want 10k non-headless Chrome browsers on your computer, there isn't enough RAM in the world

Ah good point! Yeah. That is a definite issue then. I should link this thread on the code's github. The author is getting a free code review. haha.

Hahaha I'll link this entire thread to their github

Flakey code might be an advantage in this specific case.

This joke went over so many heads, it's brilliant.

I have no experience at all using Selenium (nor automated scripts that POST stuff in the web) but I could understand what the code was doing. I can follow the code just fine.

> 1. undescriptive variable names (eg. data2)

Doesn't matter. The file is 200 lines of code.

> 2. inconsistent formatting

That didn't make the code harder to read/follow.

> 3. copy pasted fragments everywhere

Meh. Really, I've seen code where everything that could be repeated was encapsulated in a function/method/class which was being used only once.

>I can follow the code just fine.

Right, I can follow the code just fine too, but OP's claim was

>well written, rather pretty code

Like you mentioned, the file is only 200 lines, so you can get away with quite a bit and still be understandable/maintainable, but that doesn't mean they're all "well written" or "rather pretty".

> undescriptive variable names

- Have you seen Carmack or Torvalds code?

> inconsistent formatting

- Needs more explanation? It's python, consistent formatting is enforced

> copy pasted fragments everywhere

- Nothing wrong with that AT ALL if you understand how it works. It's literally the fundamental premise of Github Co-pilot.

It is middle of the road.

Which means it’s probably at least 80th percentile nowadays.

Due to selection effects, I wouldn't find it too surprising that the "middle of the road" (ie. 50th percentile) of developers with a job is equal to the 80th percentile of developers seeking a job.

Well, the reason I was able to apply both "middle of the road" and "80th percentile" labels to the same code is because the former was regarding code quality and the latter was regarding code prevalence (in the same sense that scoring 50/100 on an exam doesn't necessarily put you in the 50th percentile).

As far as developer quality goes...I'm afraid I'm not qualified to comment.