Skip to content
This repository has been archived by the owner on Oct 31, 2023. It is now read-only.

Use App Definition in task_api_tests/basic_integration.py #4817

Merged
merged 3 commits into from
Nov 7, 2019
Merged

Use App Definition in task_api_tests/basic_integration.py #4817

merged 3 commits into from
Nov 7, 2019

Conversation

maaktweluit
Copy link
Contributor

No description provided.

@maaktweluit maaktweluit self-assigned this Oct 22, 2019
@maaktweluit maaktweluit force-pushed the mwu/basic-int-app-def branch 3 times, most recently from 0af4004 to 7ba5dad Compare November 5, 2019 15:48
@maaktweluit maaktweluit changed the base branch from mwu/e2e-rtm to develop November 5, 2019 15:48
@maaktweluit maaktweluit marked this pull request as ready for review November 5, 2019 15:49
mfranciszkiewicz
mfranciszkiewicz reviewed Nov 5, 2019
View reviewed changes
Copy link
Contributor

@mfranciszkiewicz mfranciszkiewicz left a comment

Choose a reason for hiding this comment

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

Nothing really important, maybe besides the first mkdir call

scripts/task_api_tests/basic_integration.py Show resolved Hide resolved
scripts/task_api_tests/basic_integration.py Outdated Show resolved Hide resolved
scripts/task_api_tests/basic_integration.py Show resolved Hide resolved
Wiezzel
Wiezzel suggested changes Nov 6, 2019
View reviewed changes
Copy link

@Wiezzel Wiezzel left a comment •
edited
Loading

Choose a reason for hiding this comment

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

The code doesn't quite match the acceptance criteria:

  • Providing a JSON file still works
  • There is an additional option to run the test on app_id

There is no option to provide app_id instead of a JSON file.

scripts/task_api_tests/basic_integration.py Show resolved Hide resolved
@maaktweluit
Copy link
Contributor Author

@Wiezzel this is last weeks issue, using a JSON only.

app_id is a draft #4856 and will be open for review when this PR is merged

@codecov
Copy link

codecov bot commented Nov 6, 2019 •
edited
Loading

Codecov Report

Merging #4817 into develop will decrease coverage by 0.94%.
The diff coverage is n/a.

@@             Coverage Diff             @@
##           develop    #4817      +/-   ##
===========================================
- Coverage    90.15%   89.21%   -0.95%     
===========================================
  Files          231      231              
  Lines        21252    21252              
===========================================
- Hits         19159    18959     -200     
- Misses        2093     2293     +200

@maaktweluit maaktweluit merged commit dd1ce5c into develop Nov 7, 2019
@maaktweluit maaktweluit deleted the mwu/basic-int-app-def branch November 7, 2019 14:16
maaktweluit added a commit that referenced this pull request Nov 7, 2019
@maaktweluit
Merge pull request #4817 from golemfactory/mwu/basic-int-app-def
f67901a 
Use App Definition in task_api_tests/basic_integration.py
@maaktweluit maaktweluit mentioned this pull request Nov 7, 2019
Merged
maaktweluit added a commit that referenced this pull request Nov 7, 2019
@maaktweluit
Merge pull request #4869 from golemfactory/mwu/b0.22/basic-int-app-def
eb84226 
b0.22: cherry-pick #4817
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@Wiezzel Wiezzel Wiezzel approved these changes

@mfranciszkiewicz mfranciszkiewicz mfranciszkiewicz approved these changes

Assignees

@maaktweluit maaktweluit

Labels
clay
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@maaktweluit @mfranciszkiewicz @Wiezzel