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

Investigate unhealthy tests #188

Open
Poundex opened this issue Oct 10, 2024 · 6 comments
Open

Investigate unhealthy tests #188

Poundex opened this issue Oct 10, 2024 · 6 comments

Comments

@Poundex
Copy link
Contributor

Poundex commented Oct 10, 2024

  • ParallelExecutionSpec - Flaky in CI
ParallelExecutionSpec > GebReportingSpec supports parallel execution at feature level FAILED
    Condition not satisfied:

    reportFileTestCounterPrefixes("SpecRunningIterationsInParallel1") == (["000"] * 2) + (1..4)*.toString()*.padLeft(3, "0")
    |                                                                 |           |    |         |           |
    [000, 000, 001, 002, 003, 004, 004]                               false       |    |         |           [001, 002, 003, 004]
                                                                                  |    |         [1, 2, 3, 4]
                                                                                  |    [000, 000, 001, 002, 003, 004]
                                                                                  [000, 000]
        at geb.spock.ParallelExecutionSpec.GebReportingSpec supports parallel execution at feature level(ParallelExecutionSpec.groovy:186)

  • PageOrientedSpec - Flaky in CI
PageOrientedSpec > verify the Page API works for '#contentName' content > verify the Page API works for 'linkUsingPageInstance' content FAILED
    org.openqa.selenium.StaleElementReferenceException: The element seems to be disconnected from the DOM.  This means that a user cannot interact with it.
    For documentation on this error, please visit: https://selenium.dev/exceptions/#stale_element_reference
    Build info: version: '4.2.2', revision: '683ccb65d6'
    System info: host: '075353d81e19', ip: '192.168.176.3', os.name: 'Linux', os.arch: 'amd64', os.version: '5.15.0-1057-aws', java.version: '11.0.24'
    Driver info: driver.version: IDataDriver
        at app//org.openqa.selenium.htmlunit.HtmlUnitDriver.assertElementNotStale(HtmlUnitDriver.java:899)
        at app//org.openqa.selenium.htmlunit.HtmlUnitWebElement.assertElementNotStale(HtmlUnitWebElement.java:591)
        at app//org.openqa.selenium.htmlunit.HtmlUnitWebElement.verifyCanInteractWithElement(HtmlUnitWebElement.java:249)
        at app//org.openqa.selenium.htmlunit.HtmlUnitWebElement.click(HtmlUnitWebElement.java:131)
        at app//geb.navigator.DefaultNavigator.click(DefaultNavigator.groovy:672)
        at app//geb.navigator.DefaultNavigator.click(DefaultNavigator.groovy:687)
        at app//geb.content.TemplateDerivedPageContent.click(TemplateDerivedPageContent.groovy:83)
        at geb.PageOrientedSpec.verify the Page API works for '#contentName' content(PageOrientedSpec.groovy:73)
  • DateInputSpec - Functionality broken in recent ChromeDriver
    It seems this test is failing due to broken functionality in recent versions of ChromeDriver, where setting a value of an <input type="datetime-local" /> does not work as expected. There is some discussion of the issue on this php-webdriver thread. There is an existing ChromeDriver ticket but it doesn't look like it's getting fixed. A common workaround seems to be using JavaScript to set the value, rather than manipulating directly, like in this example.
@Poundex
Copy link
Contributor Author

Poundex commented Oct 23, 2024

After a bit more investigation into DateInputSpec it looks like that any ChromeDriver changes there might have been were a red herring, and this issue is caused by updating Java from 8 to 11. The default precision for new Temporals changed:

Java 8:

LocalDateTime.now().toString() // 2024-10-23T11:34:27.420

Java 9+:

LocalDateTime.now().toString() // 2024-10-23T11:35:29.955881121

with the former being accepted by Chrome and the latter being rejected as malformed. This test only tests strings that are the result of a LocalDateTime's toString. If the test had been testing a string with the nanosecond precision on current master with either the Chrome v78 that's in gebish/ci:v7 or modern Chrome it still would have failed.

@jonnybot0
Copy link
Contributor

So, for the DateTimeLocalInput, we could address this by adding a default formatter.

    void setDateTime(LocalDateTime dateTime) {
        value(dateTime.format(DateTimeFormatter.BASIC_ISO_DATE))
    }

We could add method signatures to allow folks to specify their own formatter with relative ease, though I'm less confident that's needed.

    void setDateTime(LocalDateTime dateTime, DateTimeFormatter formatter) {
        value(dateTime.format(formatter))
    }

And, of course, they can always use the setDateTime(String iso8601FormattedDateTime) signature. That's part of what makes me think we probably don't need the formatter signature, though maybe it's a "nice to have".

@Poundex
Copy link
Contributor Author

Poundex commented Oct 24, 2024

Sorry I missed this comment and wrote a summary of the issue here in #194 . The date time format mentioned there is the only one that will be accepted, so I don't think the custom format is useful. We could use the formatter to truncate the fractional seconds part and this looks like it's working. The JavaScript stuff turned out to be completely unnecessary (don't know if I misunderstood something or something that used to be an issue has now been fixed) so has been removed and we're back to just calling value(). Opened an MR here #196

@paulk-asert
Copy link
Contributor

I found that configuration.DriverConfigSpec when using Firefox driver seems to not reliably work on some headless environments. I made it just return silently in headless environments. Configuring using FirefoxOptions to headless makes the test pass in CI but that defeats the purpose as these are snippets for the documentation showing what to use in a normal environment.

@jonnybot0
Copy link
Contributor

@paulk-asert - I'm okay with the "it's not headless" assumption, but that begs the question for me of whether the configuration.DriverConfigSpec is even getting run in CI. We have Xvfb running as part of the pipeline in CircleCI, so we should be capable of running in non-headless mode in CI. Not sure we are for this module, though.

If you just run ./gradlew :doc:manual-snippets:test --tests "configuration.DriverConfigSpec", the build passes but the test doesn't actually run.

Thinking through the best way to ensure we get value out of the test, my first thought was to move the test to the real-browser submodule. Trouble is, that module is set to use a ChromeDriver.

Obviously, these snippets are meant to work whether Firefox is in headless mode or not. Indeed, they do, but your driver config needs to match the environment. geb.fixture.HeadlessTestSupport seems like it's partway there, but my hunch is we ought to go a wee bit farther. I noodled around with using asciidoctor stop/start tags to use a different driver configs (see a35598d on my branch). That makes the spec so difficult to read that I think a better path would be to add a second spec that uses headless mode.

That got me thinking: what if we just put the sample code in different files? Depending on the environment, we could load either the headless file or the headed file in the test? The documented examples could read from the normal file. 4075a38 gestures at what I mean, though it doesn't quite pass yet.

I just can't shake the feeling that having a test which never actually gets run is risky, particularly for public-facing documentation. I'm going to take a break from this, but if you have any quick pointers that would turn my straw into gold, let me know. :)

@paulk-asert
Copy link
Contributor

paulk-asert commented Dec 23, 2024 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants