Skip to content

Conversation

saihaj
Copy link
Member

@saihaj saihaj commented Sep 5, 2020

Tests assertion values were little out of sync from what SWAPI returned so updated those values.

Note: Use yarn cover to run tests because yarn test fails at the moment because Flow is broken. Also yarn coveralls isn't working and will fix it once we switch to NPM #180 and update deps

@@ -99,7 +99,7 @@ describe('Film type', async () => {
'{ allFilms { edges { cursor, node { ...AllFilmProperties } } } }',
);
const result = await swapi(query);
expect(result.data.allFilms.edges.length).to.equal(7);
expect(result.data.allFilms.edges.length).to.equal(6);
Copy link
Member

Choose a reason for hiding this comment

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

Can you investigate why we have one less film?
It should be the opposite.

Copy link
Member

Choose a reason for hiding this comment

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

Can you please check if it related to #172 or happened for some other reason?

Copy link
Member Author

@saihaj saihaj Sep 7, 2020

Choose a reason for hiding this comment

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

I think that there were only 6 movies in original too. I checked the data source file from swapi.co

This makes sense if we look at commit history of the file, Episode VII – The Force Awakens was released in 2015 and the last time the owner update things was 6 years ago (2014) and didn’t really touched them (except to fix typos) so it makes sense since there were only 6 movies.

swapi.dev has issue open Juriy/swapi#8 to update movies

species: { name: 'Human' },
species: null,
Copy link
Member Author

Choose a reason for hiding this comment

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

Related to Juriy/swapi#5

@IvanGoncharov IvanGoncharov changed the title Update tests Update tests to match swapi.dev Sep 22, 2020
@IvanGoncharov IvanGoncharov merged commit 8e93290 into graphql:master Sep 22, 2020
@saihaj saihaj deleted the update-tests branch September 22, 2020 20:58
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

Successfully merging this pull request may close these issues.

2 participants