Cypress Tests Failure

[daemon1024]

Cypress Tests are failing in the github actions and It seems to be blocking most of the PRs right now in repository.

There has been some initial discussion at https://github.com/publiclab/leaflet-environmental-layers/pull/529

Specifically Purple Air Marker Layer and Owm Layer tests are failing.

Maybe related to #360 ?

Noting https://github.com/publiclab/leaflet-environmental-layers/pull/529#issuecomment-870934595, the tests initially passed when the actions were setup

[daemon1024]

cc @noi5e @jywarren

[jywarren]

OK, so let's try to reproduce this and get a more detailed trace. Can you run it locally as well?

[jywarren]

It actually looks like it crashes Chrome:

Log here

npm ERR! code ELIFECYCLE
npm ERR! errno 4
npm ERR! leaflet-environmental-layers@2.4.3 cy:run:chrome: `cypress run --browser chrome`
npm ERR! Exit status 4
npm ERR! 
npm ERR! Failed at the leaflet-environmental-layers@2.4.3 cy:run:chrome script.
npm ERR! This is probably not a problem with npm. There is likely additional logging output above.

npm ERR! A complete log of this run can be found in:
npm ERR!     /home/runner/.npm/_logs/2021-06-22T06_40_56_149Z-debug.log

Could we search for similar failures either in this repo or in Cypress? Could it simply be a timeout if it can't fetch the data? Is there another way to get a verbose log? Thank you!!

[jywarren]

And we could compare to the error seen in https://github.com/publiclab/leaflet-environmental-layers/issues/360 which was a rate limiting issue to see if it's likely related.

[daemon1024]

I am unable to reproduce it locally, everything seems to be failing for some reason. I tried to run in Gitpod but it didn't run. Ref https://github.com/publiclab/leaflet-environmental-layers/pull/529#issuecomment-866896252

[noi5e]

I am taking a look at this... It's my first time working with the leaflet-environmental-layers library, and I'm not familiar with the Cypress testing library either. So basically, there's a lot I don't yet know about this! 😅

Maybe it would be helpful to compile the testing errors from CI:

Here they are in raw text format, to make searching easier:

   1) OpenWeatherMap layer
       adds markers on click:
     AssertionError: Timed out retrying: Expected to find element: `#map-city label`, but never found it.
      at Context.eval (http://localhost:8080/__cypress/tests?p=cypress/integration/owm.spec.js:10:10)

  2) OpenWeatherMap layer
       loads correct owmloading.gif with config option set:
     ReferenceError: LEL is not defined
      at Context.eval (http://localhost:8080/__cypress/tests?p=cypress/integration/owm.spec.js:21:14)

Purple layer - markers
    ✓ adds markers on click (1158ms)
    ✓ should have the layer name added to the hash
    ✓ has default markers in default mode (38ms)
    1) has circle markers in minimal mode
    2) shows popup
    ✓ removes markers from the map and the layer name from the hash when clicked again (157ms)

  4 passing (10s)
  2 failing

  1) Purple layer - markers
       has circle markers in minimal mode:

      AssertionError: Timed out retrying: Too many elements found. Found '3', expected '2'.
      + expected - actual

      -3
      +2

      at Context.eval (http://localhost:8080/__cypress/tests?p=cypress/integration/purpleAirMarker.spec.js:33:56)

  2) Purple layer - markers
       shows popup:
     AssertionError: Timed out retrying: expected '<div.leaflet-popup-content>' to contain 'MandMnorth40'
      at Context.eval (http://localhost:8080/__cypress/tests?p=cypress/integration/purpleAirMarker.spec.js:45:38)

[noi5e]

@daemon1024 @jywarren Looks like I'm able to open up GitPod in the PR at #529

Going to examples/index, the map seems to load normally:

I can select purple and owmlayer maps from the dropdown:

How would I reproduce these failing tests manually? I think I would need to look at them in the codebase. Or please give me a step-by-step, and I can try to do it over here in GitPod.

[daemon1024]

I am not really sure about the owm issue. Regarding, purpleAirMarker. If I just enable purpleAirMarker and nothing else on the map. it seems that nothing appears on the layer and hence the test failing. I am not even sure how some of the purpleAirMarker tests are passing.

[jywarren]

I'm guessing the tests may be depending on real data from the APIs which is not actually completely predictable. For example if one sensor on PurpleAir stops recording, we only get 2 sensors in the given map area instead of 3. That's what that test seems to show; the fact that we get 2 markers means there is /some/ data flowing. If this is correct, I think we should find a location where there are a TON of purple air markers/sensors, and just test that there are more than zero. What do you think? Then, the reference to MandMNorth40 seems to be testing for a specific named sensor. Instead let's do the same as the previous test and just assert that SOME sensors show, and not zero. We could also test the format of the API response, but it's good to test that something actually displays.

As for OWM, let's see if we can find what the looked-for elements are supposed to be and take a similar approach - adjust them to more generalized elements rather than specific ones like #map-city label.

Finally, i see there's a missing LEL reference. Once we eliminate the others, let's check to see it's loading properly and if there's a load time sequencing error of some kind.

[jywarren]

So for example here, it looks like it expects to find a city marker at the given location, to be able to click it, and to see it open and contain an image:

https://github.com/publiclab/leaflet-environmental-layers/blob/2f3723a12e70009120500f3b7603680ec7d3cbe8/cypress/integration/owm.spec.js#L3-L9

We can even test that manually for comparison using the given coordinates: /example/index.html#lat=41.6283&lon=-91.7235&zoom=10&layers=Standard

Here, in PurpleAir, we really seem to be looking for a specific marker. But since data is live, sensors can go down. Best look at purple air's website to find a sensor-dense area (a big city where the company is based maybe?) and rely on that to have a lot of sensors around:

https://github.com/publiclab/leaflet-environmental-layers/blob/2f3723a12e70009120500f3b7603680ec7d3cbe8/cypress/integration/purpleAirMarker.spec.js#L46

[jywarren]

So @daemon1024 i think here, let's go through these one by one in a PR. Let's start with an easy one -- this line references a specific sensor that's no longer present, we assume:

https://github.com/publiclab/leaflet-environmental-layers/blob/2f3723a12e70009120500f3b7603680ec7d3cbe8/cypress/integration/purpleAirMarker.spec.js#L46

So let's change it to just use a CSS class for /any/ sensor. You can load the map, visit some place, then inspect the HTML DOM and see if you can get a look at the structure of the HTML inside the .leaflet-popup-content element. Let's just change that one line to be more general - not tied to the specific name.

Once we confirm in a PR that that specific error is gone, let's repeat in a new commit with another error.

  AssertionError: Timed out retrying: Too many elements found. Found '3', expected '2'.

This one expected 2 sensors. But it turned out there are 3 there now, because this is a live API and new people put new sensors online. So let's just change the test to assert/expect that there should be more than zero.

How does that sound?

[jywarren]

We'll solve them one at time and as they get more difficult to solve, I, @noi5e and @Tlazypanda can help with the last couple. 🎉

[daemon1024]

Hey @jywarren, thanks for the detailed explanation. I have understood it and I will try to make the required modification.

Checking for more than zero rather than an exact number makes sense.

Thanks again 🙌 I will get to working on it.

[daemon1024]

Regarding the OWM failures,

Just realised that,

      var LEL = L.LayerGroup.EnvironmentalLayers({});
      LEL.addTo(map);

is being converted to

      L.LayerGroup.EnvironmentalLayers({}).addTo(map);

So hence LEL is not defined. We will need to modify our test.

I am not really sure how to access the information now though 🤔

[daemon1024]

Noting PurpleAirMaker tests passing now! Ref #533 cc @jywarren. Thank you for the pointers on the potential fixes.

[daemon1024]

About the other OWM test, it seems to be looking for city but I see which seems to be due to error ig, referencing code https://github.com/publiclab/leaflet-environmental-layers/blob/d956ceea01c9bceddd7bcce17a98ccefd38e4b66/example/layers.js#L144-L146

[daemon1024]

Also I can see Failed to fetch data! in the console. Which is related to tiles only so suspecting if the city thing we are checking is broken.

[jywarren]

So hence LEL is not defined. We will need to modify our test.

Is this in the test itself, we are overwriting LEL? What line exactly?

For the Failed to fetch data, can you either audit the outgoing requests the tests are making to see if we can manually confirm there is data, or look at the coordinates the tests are looking for and try manually navigating to that bounding box on the OWM API or web interface to visually check that there is data? It could be as easy as changing the coordinates to another city perhaps?

[daemon1024]

https://github.com/publiclab/leaflet-environmental-layers/blob/42db74ca5104ead4569ee88f3c96a0941988a7b5/cypress/integration/owm.spec.js#L17

Here we are checking for the gif url inside LEL.

[daemon1024]

https://api.openweathermap.org/data/2.5/box/city?APPID=4c6704566155a7d0d5d2f107c5156d6e&cnt=300&format=json&units=metric&bbox=-172.79296875,7.885147283424331,136.58203125000003,65.5129625532949,10

this is the url we are making request to for cities. And it's resulting in {"cod":"400","message":"Requested area is larger than allowed for your account type (25.00 square degrees)"}

I think they introduced limits for free plans? I am not sure what's the plan we are using.

[daemon1024]

https://openweathermap.org/current#rectangle

Yes there's a limit now.

[daemon1024]

So I ran the tests with a different owm map and some modifications. It passes now. But yeah we need to look into the city map for sure.

Should we try to fix it in this PR or move it to a seperate issue?

[daemon1024]

Seems like the LEL problem I mentioned. I was in the wrong, I tested on a outdated branch 😅

[daemon1024]

Fixed the tests 😄. Looking forward to your review now. Ref https://github.com/publiclab/leaflet-environmental-layers/pull/533